-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
Hosting startup topic updates #7678
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[incomplete review, to-be-continued]
|
||
*HostingStartupApp/Pages/Index.cshtml.cs*: | ||
|
||
[!code-csharp[](platform-specific-configuration/samples/2.x/HostingStartupSample/HostingStartupApp/Pages/Index.cshtml.cs?name=snippet1&highlight=11-12,19-20)] | ||
|
||
## Discover loaded hosting startup assemblies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related: The entry assembly or the assembly containing your Startup class is scanned by default for HostingStartup attributes. https://github.com/aspnet/Hosting/blob/864caa20b43ab33415795bd4f6c6d3a4365c8bfe/src/Microsoft.AspNetCore.Hosting/Internal/WebHostOptions.cs#L36-L37
@@ -16,12 +16,16 @@ public IndexModel(IConfiguration config) | |||
} | |||
|
|||
public string[] LoadedHostingStartupAssemblies { get; private set; } | |||
public string ServiceKey_Development { get; private set; } | |||
public string ServiceKey_Production { get; private set; } | |||
|
|||
public void OnGet() | |||
{ | |||
LoadedHostingStartupAssemblies = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why isn't all of this done in the constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. I'll change it on the next commit.
|
||
*HostingStartupApp/Pages/Index.cshtml.cs*: | ||
|
||
[!code-csharp[](platform-specific-configuration/samples/2.x/HostingStartupSample/HostingStartupApp/Pages/Index.cshtml.cs?name=snippet1&highlight=11-12,19-20)] | ||
|
||
## Discover loaded hosting startup assemblies | ||
|
||
To discover hosting startup assemblies loaded by the app or by libraries, enable logging and check the application logs. Errors that occur when loading assemblies are logged. Loaded hosting startup assemblies are logged at the Debug level, and all errors are logged. | ||
|
||
The sample app reads the [HostingStartupAssembliesKey](/dotnet/api/microsoft.aspnetcore.hosting.webhostdefaults.hostingstartupassemblieskey) into a `string` array and displays the result in the app's Index page: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we have this sample, this is not something we expect apps to do in code. Also, after aspnet/Hosting#1243 this list isn't very accurate. Also see my prior comment about your entry assembly being added to the list by default. Describe the config keys but remove the LoadedHostingStartupAssemblies code sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was just for demo purposes. I'll remove it on the next commit.
[EDIT] There was possibly a troubleshooting aspect to my thinking on this, too. This shows that if they just look at _config[WebHostDefaults.HostingStartupAssembliesKey]
they'll see the assemblies. I'll still cut it ... but just say'in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is "this list?"
@Tratcher Had a few minutes here to get what you requested thus far ... it's on the last commit. Let me know what u meant by "this list." What list? |
885eb34
to
d3c666d
Compare
|
||
[!code-csharp[](platform-specific-configuration/samples/2.x/HostingStartupSample/HostingStartupLib/ServiceKeyInjection.cs?name=snippet1)] | ||
|
||
The app's `Startup` class file specifies a [HostingStartup](/dotnet/api/microsoft.aspnetcore.hosting.hostingstartupattribute) attribute for the class library's `ServiceKeyInjection` class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is atypical. Adding a direct reference from your own Startup class defeats the purpose, you could already have called that implementation directly. More commonly the app would not add the attribute to their own project, it would be in an unreferenced assembly and discovered via environment variable.
The reason we search the entry assembly is that some tooling adds HostingStartup attributes dynamically at compile time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fails to describe the HostingStartup assembly attribute that can point to types in existing referenced libraries
I knew "referenced library" could've meant that the lib isn't in the same solution. Perhaps, I misunderstood @davidfowl's "very simple attribute" remark and his link to the repo sample to mean that the app would provide the direct reference from the Startup class ... that's what the linked repo sample shows.
unreferenced assembly and discovered via environment variable.
.... so .....
- Have the lib as a standalone project
- Compile the standalone lib
- bin-deploy the lib in the app
- Set the env var (
ASPNETCORE_HOSTINGSTARTUPASSEMBLIES
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the approach we use.
One other flow may be practical to include:
- Have the lib as standalone project with its HostingStartup attribute.
- Reference that project as a dependency (typically as a nuget)
- Set the env var (ASPNETCORE_HOSTINGSTARTUPASSEMBLIES)
This allows nuget packages to provide startup functionality without any code changes, but you still have a compile time dependency so that removes all of the deps and deployment work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok ... sounds good. I'll work on this tomorrow, and let's look again next week.
The sample app reads the [HostingStartupAssembliesKey](/dotnet/api/microsoft.aspnetcore.hosting.webhostdefaults.hostingstartupassemblieskey) into a `string` array and displays the result in the app's Index page: | ||
|
||
[!code-csharp[](platform-specific-configuration/sample/HostingStartupSample/Pages/Index.cshtml.cs?name=snippet1&highlight=14-16)] | ||
Hosting startup assemblies are listed in the [WebHostDefaults.HostingStartupAssembliesKey](/dotnet/api/microsoft.aspnetcore.hosting.webhostdefaults.hostingstartupassemblieskey). Excluded assemblies are listed in the [WebHostDefaults.HostingStartupExcludeAssembliesKey](/dotnet/api/microsoft.aspnetcore.hosting.webhostdefaults.hostingstartupexcludeassemblieskey). For more information, see [Web Host: Hosting Startup Assemblies](xref:fundamentals/host/web-host#hosting-startup-assemblies) and [Web Host: Hosting Startup Exclude Assemblies](xref:fundamentals/host/web-host#hosting-startup-exclude-assemblies). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of assemblies to search for HostingStartup attributes is loaded from config under the HostingStartupAssemblies key. The list of assemblies to exclude from discovery is loaded from config under the HostingStartupExcludeAssemblies key.
@Tratcher I updated the topic+sample along the lines of your feedback. It has a standalone class lib scenario and a NuGet package scenario now. I go with the NuGet scenario first, as I think that might be more popular. The paint is still wet on these updates, so I'll make anther pass Monday morning. Three items came up along the way:
|
@Tratcher I'm sure it needs more work, but take a 👀 now. There were a lot of updates this round. |
<xref:fundamentals/host/hosted-services> | ||
Learn how to implement background tasks with hosted services in ASP.NET Core. | ||
|
||
<xref:fundamentals/configuration/platform-specific-configuration> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configuration/ -> host/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rick-Anderson
issued a ruling that UIDs are not allowed to change when topics are moved.
Learn how to implement background tasks with hosted services in ASP.NET Core. | ||
|
||
<xref:fundamentals/configuration/platform-specific-configuration> | ||
Discover how to enhance an ASP.NET Core app from a class library or external assembly using an `IHostingStartup` implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between a class library and an external assembly? You mean from assemblies your app may or may not reference? Change class library to referenced assembly and external assembly to unreferenced assembly?
|
||
To discover hosting startup assemblies loaded by the app or by libraries, enable logging and check the application logs. Errors that occur when loading assemblies are logged. Loaded hosting startup assemblies are logged at the Debug level, and all errors are logged. | ||
|
||
## Disable automatic loading of hosting startup assemblies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should ASPNETCORE_HOSTINGSTARTUPASSEMBLIES be discussed before Disable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These sections originally ended up here due to your feedback at #4967 (review) ...
Let's move these new sections to the very top right after the opening paragraph, they are applicable to the most users. Comparatively few users implement this feature.
I'm not sure it matters. All it's really saying is list them in this env var and they go away. ASPNETCORE_HOSTINGSTARTUPASSEMBLIES
isn't directly relevant to the goal of this section, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just though it was weird you said how to turn the feature off before you said how to turn it on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I first added it, it was way down at the bottom just above the last section on the sample app.
If you want me to move the disabling part back down there, let me know. We can tack-on a sentence to the discovery section that says ...
To disable hosting startup enhancements, see <bookmark>.
|
||
Each approach is described in the following sections. | ||
|
||
## NuGet package activation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nuget packages are not treated specially for this feature, they're only a mechanism for acquiring the class library dependency. This can be added as a note in the class library section, it doesn't need its own section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are not treated specially for this feature
I understand that from the engineering perspective (i.e., delivery is a separate issue), but we're dealing with a slightly complicated scenario overall with some subtle differences between the gross implementation approaches (gross = including delivery).
It's easier to organize the topic content for clarity on the differences by going with separate (but similar) sections due the differences in procedure. This leaves the reader simply to ask, "How do I want to supply this?" Then, they read the applicable section. Otherwise if they aren't sure, they can read them in a more SxS fashion and easily compare and contrast the two.
If we roll NuGet into the class lib section (and remove the parts of the sample that pertain to the NuGet approach ... probably water down the text to basic 'use NuGet to deliver it' statements), I think the gross implementation will become harder to understand.
It's your call ... I'll cut it if you want, but I don't recommend it. If you want to proceed with cutting/moving, do you want it out of the sample, too?
Just remember as u make your final call, I didn't write this for you ... I wrote it for ME. 😄 lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you keep it then there are some parts that need to be fixed up. E.g. you say to set the environment variable to packageid, but the env var always needs to use the assembly name which may not match the packageid.
|
||
[!code-csharp[](platform-specific-configuration/samples/2.x/HostingStartupApp/Pages/Index.cshtml.cs?name=snippet1&highlight=7-8,13-14)] | ||
|
||
The compiled class library is bin-deployed to the app, and the class library's assembly name is listed in the `ASPNETCORE_HOSTINGSTARTUPASSEMBLIES` environment variable. The environment variable is a semicolon-delimited list of assemblies that contain a `HostingStartup` attribute that identifies a hosting startup enhancement to load. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't really matter where the binary is in this case, so long as the application has a compile time reference to it. E.g. it could be in the runtime store, in the bin directory, etc., so long as it's in one of the normal runtime search paths.
|
||
## Assembly activation | ||
|
||
This section describes how to activate a hosting startup from a bin-deployed or runtime store-deployed assembly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from an assembly that you do not have a compile time reference for. This is something a hosting provider like azure might provide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will only work for .NET Core, not .NET Framework. The direct dependency approach works on both.
|
||
### Create the assembly | ||
|
||
An `IHostingStartup` enhancement is deployed as an assembly based on a console app without an entry point. The assembly references the [Microsoft.AspNetCore.Hosting.Abstractions](https://www.nuget.org/packages/Microsoft.AspNetCore.Hosting.Abstractions/) package: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helps it generate the correct deps file (correct?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the wrapper project and getting the right deps file came about from @pakrym
's comment here 👉 #4438 (comment) (referring to answering me on 4 and 7).
What's the deal with the hosting startup on ...
- Compile-time reference present: No deps file needed (no additional deps specification)
- No compile-time reference: Deps file needed (additional deps specification required)
If you can help me understand that, I think a few words in the topic on it would help readers understand the requirement in the no-compile-time reference scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core will only load assemblies listed in a deps file. When you have a compile time reference then the dependent assembly and all of its own dependencies end up in the app deps file. When you don't have a compile time reference then you need to provide a supplementary deps file with all of that assemblies dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Under the class lib approach if the class lib's assembly is placed into the runtime store, is an explicit compile-time reference required in the consuming app's project file?
I have it bin-deployed in the text and sample app with a reference in the sample app's project file ..
<ItemGroup>
<Reference Include=".\bin\Debug\netcoreapp2.1\HostingStartupLibrary.dll">
<HintPath>.\bin\Debug\netcoreapp2.1\HostingStartupLibrary.dll</HintPath>
<SpecificVersion>False</SpecificVersion>
</Reference>
</ItemGroup>
I'm just wondering in the runtime store case what the compile-time reference looks like. We may need to add it. [EDIT] ... add it in-text ... not in the sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it doesn't require a compile time reference, just an additional deps. That's how we ended up using it for AppInsights and AppServices. Go look at C:\Program Files\dotnet\additionalDeps\Microsoft.AspNetCore.AzureAppServices.HostingStartup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot of overlapping parts between the scenarios. What if we start by explaining the smaller components and then pull them together in the two main scenarios at the end? And those scenarios build on eachother. E.g. the no-compile-time-reference scenario is an expansion on top of the compile time reference scenario that uses all the same mechanics and adds the additional deps.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that sounds good (devil might be in the details tho). I'll take a good night sleep + up early + plenty of ☕️ and see how it goes in the morning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to know this tho -------
What's the practical difference between:
- A class lib in the runtime store with additional deps
- A console app without an entry point in the runtime store with addtional deps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pakrym ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently if the console app w/o entry point assembly is bin-deployed with a compile-time ref, there's no need to deal with the deps file (just like in the console lib assembly case).
Assembly type (delivery options)
- Class lib in a local assembly (runtime store, bin, or NuGet)
- Console app w/o entry point (runtime store, bin, or NuGet - can it be packed?)
Delivery options
- runtime store without compile-time ref and provide deps
- bin with compile-time ref
- NuGet package with compile-time ref
Outline
🥁 roll plz ...
[This outline is in the order of presentation.]
- Class lib (only how to create the assembly here)
- Console app w/o entry point (only how to create the assembly here)
- Activation
- List in
ASPNETCORE_HOSTINGSTARTUPASSEMBLIES
- Runtime store
- Where to place assembly
- Provide deps instructions
- NuGet - I still prefer to keep NuGet separate from class lib and bin-deployment. With this outline, there should be minimal duplication of text among the sections.
- Link to NuGet packing instructions (https://docs.microsoft.com/dotnet/core/deploying/creating-nuget-packages)
- No deps work required
- Deployment
- NuGet package via nuget.org (https://docs.microsoft.com/nuget/create-packages/publish-a-package)
- NuGet package in the runtime store (https://docs.microsoft.com/dotnet/core/deploying/runtime-store)
- bin with compile-time ref
- Where to place assembly
- No deps work required
- List in
- Sample app - Unlike the topic today, completely separate the sample description and steps out of the text above. Only include in the sample the most likely deployment approaches.
The pieces are just about all here, so I'm not sorry about the work that's been done thus far. However, let's settle on an outline (maybe that one ☝️) before I continue.
Posers, questions, queries
- What's the practical difference between class lib and console app w/o entry point approaches? Since it looks like they can both delivered with any delivery approach here, it comes down to the practical difference between them. Which one makes sense for given scenarios?
- Are class lib+NuGet package and console app w/o entry point+runtime store the most common anticipated assembly+delivery approaches? If so, the sample only needs to show these two.
- Between NuGet package on nuget.org and NuGet package in the runtime store, shall we just link out, or shall we link out with guidance on pros+cons?
- Can we say something about installer options? ... making delivery dead-on simple in multi-machine environments without using NuGet? ... or do we just lean in the direction of you should always use NuGet!
Something about the organization is still bugging me. The ordering seems strange and ASPNETCORE_HOSTINGSTARTUPASSEMBLIES is duplicated several times. If I'm trying to author one of these what progression would I need? How's this for a TOC?
|
I didn't do the outline work that I show in #7678 (comment). I'll take another look at what I have there and what you posted and see what the differences are. Check my Activation node in that outline tho ... I have the env var moved up very early. However, I do have creating the class lib or console app w/o entry point before that. Before I do anything tho, I probably need to hear the answers to these questions ...
cc: @pakrym |
I looked this over in comparison ...
I didn't include a complete outline before, so a couple of those would've been left in-place.
Broadly speaking, my outline does this ...
Breaking down Create, it's either ...
(I need to hear more on the scenarios where they apply in the real world so I know what to tell readers.) Breaking down Activation, it's ...
Therefore, I devised it this way (and I'll add those discovery/disable bits this time) ...
... but even if that's a good work starting point, I need the answers to those posers that I listed earlier ☝️ before I proceed. |
I've asked @pakrym for a second opinion on organization and to answer some of your technical questions. |
@Tratcher Gave it a pass this morning. It seems fine to me ... I didn't get 😵 outside of the questions I asked earlier. I ended up moving the section on the attribute to the top of the topic, but I left the "discovery" and "disable" sections immediately under it. I left the sample code as it was until I receive more feedback. [EDIT] btw -- If it makes sense to do so, this is a good time to change the file name (URL). The "platform-specific-configuration" segment might be better as "hosting-startup." I can't change the UID, but we can change this aspect. |
I think there is a bit of confusion about why exactly we have a class library and console app. Let me clarify a bit. What usually happens when we build hosting startups is first we create an implementation library (the one the has IHostingStartup) and target appropriate TFM in it (netstandard/netcoreapp) and treat it as a normal package. Problem with a class library is that we can't get additional deps file out of it (because deps files are runable application assets) and you can't generate runtime store for it either ( The last thing we need to do is to remove dummy app entry from [1] - shared runtime you target should be the same as the framework name you put into
Dynamic here means that you expand application dependency closure in runtime dynamically vs build time (at build time you do it by referencing packages).
Two largest ones are:
|
@pakrym Thanks for explaining. Yes, you were correct to clarify the dynamic approach. I can enhance the topic's dynamic content with the information you provided. You don't address the static (compile-time referencing) approach for a class lib/NuGet package. I'm attempting to follow
Focusing on ...
... and ...
Do the approaches on this PR meet those goals? RE: The practical diff between static and dynamic ... It looks like the practical diff is just that ... The dynamic approach doesn't require the compile-time ref. The static approach does. That's it (it seems). If so, that's easy enough to remark upon in the topic. For the sample updates on this PR (to match the approaches described in the topic), I have:
Is that good? |
Latest version is creating additional questions/concerns ...
If the original topic (and my current instructions are incorrect), I should've pinged you back in December. #4967 However, Hypothetical outline reversion to recombine runtime package store delivery with the dynamic approach:
|
Depends on the requirement. If the only requirement is to produce output that would work with additionalDeps/runtime store having IHostingStartup in console app is enough. Sometimes having HostingStartup Nuget package is required and aconsole app would produce an unusable package. So having package and app covers all the scenarios.
The dev. Everything I talked about was from the point of view of HostingStartup package author.
No, dev has to call |
412c2cd
to
5c7aaa4
Compare
@pakrym U cool with the latest updates? I produce a deployment folder with ...
|
@pakrym Can you review the last and hopefully final updates? |
Looks good. Awesome work @guardrex ! |
Thanks @pakrym ... that means a lot to me coming from you. |
@@ -4,7 +4,7 @@ author: guardrex | |||
description: Learn about the ASP.NET Core Web Host and .NET Generic Host, which are responsible for app startup and lifetime management. | |||
ms.author: riande | |||
ms.custom: mvc | |||
ms.date: 05/16/2018 | |||
ms.date: 07/27/2018 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
07 --> 08
Learn how to implement background tasks with hosted services in ASP.NET Core. | ||
|
||
<xref:fundamentals/configuration/platform-specific-configuration> | ||
Discover how to enhance an ASP.NET Core app from a referenced or unreferenced assembly using an `IHostingStartup` implementation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove the extra space after "referenced".
- Link to the IHostingStartup API ref
To disable automatic loading of hosting startup assemblies, use one of the following approaches: | ||
|
||
* To prevent all hosting startup assemblies from loading, set one of the following to `true` or `1`: | ||
- [Prevent Hosting Startup](xref:fundamentals/host/web-host#prevent-hosting-startup) host configuration setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert dashes to asterisks
|
||
1. An implementation library is created from the class that contains the `IHostingStartup` implementation. The implementation library is treated as a normal package. | ||
1. A console app without an entry point references the implementation library package. A console app is used because: | ||
- A dependencies file is a runnable app asset, so a library can't furnish a dependencies file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use asterisks instead of dashes
This command places the hosting startup assembly and other dependencies that aren't part of the shared framework in the user profile's runtime store at: | ||
|
||
``` | ||
<DRIVE>\Users\<USER>\.dotnet\store\x64\<TARGET_FRAMEWORK_MONIKER>\<ENHANCEMENT_ASSEMBLY_NAME>\<ENHANCEMENT_VERSION>\lib\<TARGET_FRAMEWORK_MONIKER>\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path looks specific to Windows. What about macOS and Linux?
If you desire to place the assembly and dependencies for global use, add the `-o|--output` switch to the `dotnet store` command with the following path: | ||
|
||
``` | ||
<DRIVE>\Program Files\dotnet\store\x64\<TARGET_FRAMEWORK_MONIKER>\<ENHANCEMENT_ASSEMBLY_NAME>\<ENHANCEMENT_VERSION>\lib\<TARGET_FRAMEWORK_MONIKER>\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is Windows specific again. I recommend using OS tabs.
The [sample code](https://github.com/aspnet/Docs/tree/master/aspnetcore/fundamentals/host/platform-specific-configuration/samples/) ([how to download](xref:tutorials/index#how-to-download-a-sample)) demonstrates three hosting startup implementation scenarios: | ||
|
||
* Two hosting startup assemblies (class libraries) set a pair of in-memory configuration key-value pairs each: | ||
- NuGet package (*HostingStartupPackage*) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convert dashes to asterisks
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.AspNetCore.App" Version="2.1.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade to 2.1.3
nuget.org. | ||
--> | ||
<PropertyGroup> | ||
<RestoreSources>$(RestoreSources);https://api.nuget.org/v3/index.json;../HostingStartupPackage/bin/Debug</RestoreSources> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this typically defined in a NuGet.config file? What's the advantage of putting it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how typical it is. It works either way. I like this approach because it applies the sources only to the current project. I think everyone has had the unfortunate experience where a Nuget.config messes up the SDK or the sources for some downstream project. If they want that, it's great. When they don't and it isn't clear where the config is coming from, it's very frustrating. I don't think that would be a problem here, but this approach is supported.
@dasMulli ... Do you have any thoughts on the two approaches? From your research, do you think MS has pushed devs toward one approach over the other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things here:
-
pointing to a bin/Debug output as restore source is a bit hacky but I don't know the context here. (what if it fails, what if NuGet decides that you haven't changed any feeds/references and does a no-op (incremental) restore etc.)
-
There's also
RestoreAdditionalProjectSources
for project-specific sources. But now idea how well a "manage package for solution" dialog works for this.
The property-based approach is nice if you don't want to explain NuGet.config and the mechanism behind and it is easy to set in Directory.Build.props
for all projects, even for local paths by using $(MSBuildThisFileDirectory)local\nupkgs
from it. Also, you can make use of standard MSBuild conditions to alter the restore sources which is why dotnet/aspnet moved to them to be able to use different sources for different types of builds which would require editing NuGet.config for every build otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True ... it's a bit of a hack. It has the following benefits/goals in this context:
- It allows the dev to create and use the HostingStartupPackage NuGet package in the HostingStartupApp by just building that HostingStartupPackage NuGet package project and then building/running the HostingStartupApp.
- They don't need to move anything (i.e., they don't need to move the NuGet package out of the HostingStartupPackage project). They don't need to take any additional steps in HostingStartupApp to find and consume the package.
- They don't need to put the package on nuget.org. That's the really important goal here.
you haven't changed any feeds/references and does a no-op (incremental) restore
I thought that if the NuGet package is rebuilt that NuGet will pick up on that (without a package version change). Does NuGet go purely by the package's version on incremental restore? That is a problem if that's the case. I would like to have this set up so that the dev can make a change to the NuGet package project and have it picked up on restore/run of the HostingStartupApp.
RestoreAdditionalProjectSources
Can the additional project source be a NuGet package that is then consumed by the app? If I can use RestoreAdditionalProjectSources
pointed to the NuGet package project, that would be great ... but is that going to work for consuming a NuGet package? If so, do I then drop the <RestoreSources>
property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@guardrex AFAIK you'd have to do a dotnet nuget locals all --clear
if you change package contents without changing the version anyway - regardless of which feed the package is coming from (directory, web feed).
That's why you should always use project (P2P) references during development or use a local "global packages cache" to nuke in between full rebuilds (CI runs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a project reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use a project reference?
Because the app consuming a hosting startup should not have a project reference (for this particular scenario ... for the 'consume it from a package scenario') ... it should have an actual NuGet package reference to demo this. That's the way the app would actually consume the hosting startup.
Therefore, I wouldn't mind it if a project reference did produce a real NuGet package. It sounds like you're saying tho that it won't do that and that the consuming app wouldn't also have a regular package reference, too.
Therefore, I think I just need to remind readers that if they change the package project and recompile it that they'll also need to execute a cache clearing as you say to make sure that the project that consumes the package will get a fresh copy from the package project.
As for the <RestoreSources>
versus Nuget.config situation, I still feel like this should stick with <RestoreSources>
. It's simpler this way, and it's well documented in the consuming project's project file. I doubt that devs getting geared up to use a hosting startup will have any problem with this approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dasMulli ... I pushed a commit to make sure devs know that if they change the package project to clear caches. That looks good.
I'm going to leave the <RestoreSources>
in the app as is. Unless @pakrym says 'don't do that,' it seems like a very clean, easy-to-use, easy-to-grok setup for the demo. I'm just trying to show how the hosting startup consumer takes a real package, and I don't want the dev to have to put a hosting startup package on nuget.org or need to sweat a Nuget.config file with the source (which I think buries how the source is being set).
Your advice on the caches should go well 👍. They just need to do that each time they make changes to the hosting startup package. 🤞
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="Microsoft.AspNetCore" Version="2.1.2" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade to 2.1.3
@scottaddie @pakrym These look correct based on my iMac research. For the user profile versions ...
For the machine-wide versions ...
I also updated the Windows ones to use env vars ...
|
@scottaddie Unless there's anything else, I think we're in good shape here. The paths for Mac/Linux seem good based on my iMac research here at the house. I'm sure the community will come for my head 🔪 👦 if I made a boo boo, but my iMac is saying these are good. (We'll blame my iMac if they're wrong. 🙈) |
Thanks again to @dasMulli! 🚀 🚀 🚀 ... the tip for clearing local caches is critical for the dev who wants to play with the NuGet package approach and get their hosting startup package updated in the app. That was a 🎸 rockstar suggestion 🎷. |
|
||
### Console app without an entry point | ||
|
||
*This approach is only available for .NET Core apps, not .NET 4.x.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.NET 4.x.* --> .NET Framework
|
||
* [Runtime store](#runtime-store) – Activation doesn't require a compile-time reference for activation. The sample app places the hosting startup assembly and dependencies files into a folder, *deployment*, to facilitate deployment of the hosting startup in a multimachine environment. The *deployment* folder also includes a PowerShell script that creates or modifies environment variables on the deployment system to enable the hosting startup. | ||
* Compile-time reference required for activation | ||
- [NuGet package](#nuget-package) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use asterisks here instead of dashes?
|
||
## Activation | ||
|
||
Three options for hosting startup activation are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see 2 options listed here.
|
||
--- | ||
|
||
If you desire to place the assembly and dependencies for global use, add the `-o|--output` switch to the `dotnet store` command with the following path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch --> option
|
||
The location of the implementation's *\*.deps.json* file is listed in the `DOTNET_ADDITIONAL_DEPS` environment variable. | ||
|
||
If the file is placed in the user profile's *.dotnet* folder for per-user use, set the environment variable's value to: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "for per-user use". That's implied when you reference "user profile" earlier in the sentence.
|
||
--- | ||
|
||
For the sample app (*HostingStartupApp*) to find the dependencies file (*HostingStartupApp.runtimeconfig.json*), the dependencies file is placed in the user's profile. and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and
|
||
For more information on NuGet packages and the runtime store, see the following topics: | ||
|
||
* [How to Create a NuGet Package with Cross Platform Tools](https://docs.microsoft.com/dotnet/core/deploying/creating-nuget-packages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the "https://docs.microsoft.com" prefix from these 3 links.
|
||
## Sample code | ||
|
||
The [sample code](https://github.com/aspnet/Docs/tree/master/aspnetcore/fundamentals/host/platform-specific-configuration/samples/) ([how to download](xref:tutorials/index#how-to-download-a-sample)) demonstrates three hosting startup implementation scenarios: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should "three" be "two"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to the problem you pointed about above: There are ... for lack of a better expression ... three options that call under two approaches. ei ei ei. I think the word "three" can be removed to solve it most easily.
|
||
**Activation from a runtime store-deployed assembly** | ||
|
||
1. The *StartupDiagnostics* project uses [PowerShell](/powershell/scripting/powershell-scripting) to modify its *StartupDiagnostics.deps.json* file. PowerShell is installed by default on Windows OS starting with Windows 7 SP1 and Windows Server 2008 R2 SP1. To obtain PowerShell on other platforms, see [Installing Windows PowerShell](/powershell/scripting/setup/installing-windows-powershell). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OS- I suspect you want to link here instead in that last sentence: /powershell/scripting/setup/installing-powershell#powershell-core
```console | ||
dotnet store --manifest StartupDiagnostics.csproj --runtime win7-x64 | ||
``` | ||
For Windows, the command uses the `win7-x64` runtime identifier (RID). When providing the hosting startup for a different runtime, substitute the correct RID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link the RID text to the following doc: /dotnet/core/rid-catalog
For Windows, the command uses the `win7-x64` runtime identifier (RID). When providing the hosting startup for a different runtime, substitute the correct RID. | ||
1. Set the environment variables: | ||
* Add the assembly name of *StartupDiagnostics* to the `ASPNETCORE_HOSTINGSTARTUPASSEMBLIES` environment variable. | ||
* On Windows, set the `DOTNET_ADDITIONAL_DEPS` environment variable to `%UserProfile%\.dotnet\x64\additionalDeps\StartupDiagnostics\`. On macOS/Linux, set the `DOTNET_ADDITIONAL_DEPS` environment variable to `/Users/<USER>/.dotnet/x64/additionalDeps/StartupDiagnostics/`, where `<USER>` is the user profile that contains the hosting startup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to introduce OS tabs here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed the command by showing <RID>
. Therefore, there's only one sentence left specific to non-Windows ... this one. Let's see on the next commit. It might not be worth it to copy the entire set of steps into three tabs only to accommodate the single sentence.
from the app's bin folder. | ||
--> | ||
<ItemGroup> | ||
<Reference Include=".\bin\Debug\netcoreapp2.1\HostingStartupLibrary.dll"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dasMulli,
It does seem hacky to hardcode certain pieces of this path. Should they be converted to variables? For example:
- Debug --> $(Configuration)
- netcoreapp2.1 --> $(TargetFramework)
- HostingStartupLibrary.dll --> $(AssemblyName)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a great idea. We do that elsewhere, and we probably should do it here, too.
I'll use different names. They refer to characteristics of another project (e.g., the hosting startup package project, the hosting startup lib). I'll make it clear in their names on the next commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk on this now ....... this is taking something very simple and making it very messy. It takes three vars for both to define them. It makes it harder to see what it is .... just a path. I'd rather be super clear on the components of the path than have a bunch of vars stuck in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So.. one is a package reference and the other a Reference
instead of a project-to-project reference..
I'd argue that the Reference
should really be a ProjectReference
instead, so it is built, up-to-date and you get the correct path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw i'd prefer <Reference Include="path/to/some.dll" />
instead of the hint path approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that @scottaddie ... I just noticed that you are correct about the instructions and location for the hosting startup library. The instructions are correct ... I didn't recall that I have the dev deploy the assembly to the consuming project. These scenarios aren't exactly easy to track. I'll add project file language to reinforce the concept, and I'll add the vars you recommended.
☕️ ....... one more cup please!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug --> $(Configuration)
netcoreapp2.1 --> $(TargetFramework)
HostingStartupLibrary.dll --> $(AssemblyName)
All of these only apply when the referenced project is built under the exact same parameters.
$(Configuration)
-> Probably okay if they are built in the same solution and users didn't mess with solution configurations.$(TargetFramework)
-> as long as the library isn't anetstandard*
...$(AssemblyName)
-> only if the referenced project's.csproj
file has the exact same name as the currently built project.. don't think that will happen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these only apply when the referenced project is built under the exact same parameters.
It's just for the path tho (within the consuming project; I mean the configuration and the target framework vars), so the path to the DLL should be correct.
I need to think this over a little more. Everything we've discussed has merit, but this is on a slippery slope to being so ✨ magical ✨ that it clouds the basic concepts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's also..
<ProjectReference Include="../some/other.csproj"
ReferenceOutputAssembly="false" />
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fleshing out some details in the app's project file to explain what's going on.
I may also mirror some or all of the new remarks in the topic itself.
It will be on the next commit.
Ok ... here's what I came up with. I provide additional notes to hopefully make it clear what's going on in the project file. I don't want those notes in the topic. These are sample demo implementation details. They're a bit beyond the scope of the hosting startup basic concepts. The topic states plainly what the options are and links out where needed (e.g., NuGet packages). I add a couple of lines where the deployment options are described explaining the circumstances where the two main compile-time approaches apply. In the first case, it would be a NuGet package published to nuget.org. In the lib case, it would be a compiled assembly moved to some location where an app can take a compile-time ref on its assembly. Let's deal with any confusion that arises in the community as we get feedback. I'm sure they'll let me know if things turn sour. |
Fixes #6896
Internal Review Topic
AddInMemoryCollection
.ASPNETCORE_HOSTINGSTARTUPEXCLUDEASSEMBLIES