dotnet store cross-gen all versions of a dependency

[JunTaoLuo]

From my understanding, the current dotnet store restores and cross-gens each package separately and this could lead to multiple versions of the same dependency in the store, even though we envision only one version being loaded at runtime.

This is better illustrated by the Microsoft.AspNetCore.Hosting package in the sample repro I have created at https://github.com/JunTaoLuo/StoreMultiVersion. The dependency is brought in via Microsoft.ApplicationInsights.AspNetCore:2.0.0 -> Microsoft.AspNetCore.Hosting:1.0.0 and Microsoft.AspNetCore.Server.Kestrel:2.0.0-preview1-final -> "Microsoft.AspNetCore.Hosting:2.0.0-preview1-final. This leads to both the 1.0.0 and 2.0.0-preview1-final versions of Hosting to be produced in the cache in the .out/ directory of the repro.

We envision users to consume all of our packages through the metapackage which means the version of Hosting will always resolve to 2.0.0-preview1-final, see the obj/project.assets.json and the *.deps.json files. The proposal here is that since only the 2.0.0-preview1-final version is used by the app, we should only produce stores with that resolved version by mimicking the logic in restore. Does this make sense?

cc @gkhanna79 @eerhardt @ramarag

[gkhanna79]

The proposal here is that since only the 2.0.0-preview1-final version is used by the app, we should only produce stores with that resolved version by mimicking the logic in restore

Store command has not inherent knowledge of which package will be required at runtime and should not be in the business of determining that as well. The only entity who knows this is the store profile manifest author who generates the store and they should use such knowledge to figure out what should be present or not.

[eerhardt]

The store command creates the fully expanded graph of dependencies for the packages you specify. There is no dependency version unification between separately specified packages.

The reasoning is because if someone just references Microsoft.ApplicationInsights.AspNetCore:2.0.0 in their app, and publishes against a store manifest that contains Microsoft.ApplicationInsights.AspNetCore:2.0.0 in it. The person's app will be transitively referencing Microsoft.AspNetCore.Hosting:1.0.0, because that is the version that was brought in by Microsoft.ApplicationInsights.AspNetCore:2.0.0. If the store command didn't put Microsoft.AspNetCore.Hosting:1.0.0 in the store, you'd get a behavior where one package (ApplicationInsights.AspNetCore) came from the store, but that package's dependency (AspNetCore.Hosting) needed to be published app-local.

[JunTaoLuo]

We are not concerned about the scenario of users referencing the M.Application.AspNetCore:2.0.0 package directly, and having it published app-local seems reasonable to me. In fact, we reference that package here along with M.A.Hosting (https://github.com/aspnet/AzureIntegration/blob/5d162d79e8edf195877088e530aa2a16c14915e2/src/Microsoft.AspNetCore.ApplicationInsights.HostingStartup/Microsoft.AspNetCore.ApplicationInsights.HostingStartup.csproj#L15-L16) which means the version of Hosting is always overwritten to 2.0.0 and version 1.0.0 is never used (unless specifically specified by the user). In an effort to reduce the number of redundant versions of the store, we were hoping to have dotnet store use the same version unification logic as dotnet restore.

[JunTaoLuo]

FYI @DamianEdwards @davidfowl

[ramarag]

Why do you refer M.A.Hosting 1.0 ever in M.Application.AspNetCore:2.0.0. Should you not be removing the reference there.

From the store user's perspective the App actually might not want to use M.A.Hosting 2.0 and actually downgrade it to some other version of M.A.Hosting that is the lowest common denominator.

[JunTaoLuo]

We don't ship M.ApplicationInsights.AspNetCore. That's created by the AppInsights team and they depend on our LTS 1.0.0 versions. However, in our store we would prefer hoisting up to the latest versions of our packages. If the user would like to downgrade then they will just end up with the older versions of our packages in the app local output.

My understanding is that stores are composable. If you want to, you can install the 1.0.0 store (hypothetically, but we won't be building it) and the 2.0.0 store and trim by the manifest of both. Otherwise the 2.0.0 store should only contain 2.0.0 packages and any 1.0.0 packages will be published app-local.

[ramarag]

This is the shortcoming of how the packages are specified, if we are to take a unit of dependency then nuget will compute the correct package graph. From the generic store functionality we have two options:

1. Specify the file as unit of dependency - This would imply multiple versions of the same package can never be put in the store using a single file.
2. Each PackageReference as the unit of dependency - which is what we have elected to do
3. Come up with a custom syntax to indicate a dependency group and one for specifying multiple versions

[ramarag]

By 3, I mean
PackageReference's will be restored as a group
Add another itemgroup called AdditionalPackageReference which will be restored individually.

We will have to take a breaking change to ensure that the ItemGroup names are consistent with their other usag

[gkhanna79]

@JunTaoLuo Why do we think this is required for 2.0? Store command's granularity is package and based upon the transitive closure of the packages specified.

[eerhardt]

An optional workaround here for 2.0 is to create a single package that references all the packages you want in the store. Then specify that single package in your manifest. The store command will only create 1 package graph for the whole store instead of restoring each package individually.

[JunTaoLuo]

I'll give that a try.

[nguerrera]

dotnet store is basically on life support right now. I'm trying to simplify it as I fix what's been broken during recent changes and to prevent it from being such a high tax on future changes. It looks like it would be drastically simpler to do option 1 above:

Specify the file as unit of dependency - This would imply multiple versions of the same package can never be put in the store using a single file.

@eerhardt @DamianEdwards @jeffschwMSFT @fadimounir Any objections to this breaking change? The result will be that what is stored is based on the normal nuget rules for a set of package references in a project file. If you want to store multiple versions, you will need to put use more than one file / dotnet store run.

[eerhardt]

Are we certain we need to fix this issue? I don't think ASP.NET uses the dotnet store feature going forward. Do we have other customers of the feature that need this to be fixed?

[JunTaoLuo]

I think we still use dotnet store in our 2.x servicing for building site extensions, is that right @natemcmaster @dougbu?

[nguerrera]

For that, you would use a 2.x sdk, though, right, so it would not be impacted by this. The change I'm proposing would impact 3.0 sdk and onward.

[JunTaoLuo]

Oh okay, yes I think that should be okay then.

[natemcmaster]

We still use dotnet store in 3.0 to generate the stores used by our Azure Apps Services extensions. https://github.com/aspnet/AspNetCore/tree/master/src/SiteExtensions

[nguerrera]

@natemcmaster Thanks for noting that. Does the change I proposed above work for that?

[nguerrera]

@normj Reaching out to you as a user of dotnet store based on https://github.com/dotnet/cli/issues/10784 Would the proposed change above work for you? Basically, if there is "incoherence" in the list of packages to store, then they would resolve in the usual nuget way for a single project.

For example, if we're asked to store:

A v1
B v2

And dependencies are:

A v1 -> B v1 -> C v1
B v2 -> C v2

Then we will store A v1, B v2, C v2 under the proposal:

Whereas before we would store A v1, B v1, C v1, B v2, C v2

If you really wanted to store multiple versions of packages, you would have to run store multiple times with different inputs.

[natemcmaster]

I think the proposal works for aspnet. I review the store we ship in Microsoft.AspNetCore.AzureAppServices.SiteExtension, and I only see one version of each package.

[normj]

For reference here is a description of how we use dotnet store at AWS for use with Lambda functions which is AWS's serverless technology.

https://aws.amazon.com/blogs/developer/aws-lambda-layers-with-net-core/

The short answer is yes this would be a breaking change that could negatively impact our users.

The intention is users will create a package store and then reference that for any number of Lambda functions they want to deploy. Those Lambda functions can use a variety of versions of the dependencies. Usually what I tell users who have a collection of functions is create a separate store manifest taking all of the packagereferences in their projects and create a store from that which will get all of the separate versions. With this change that workflow isn't guaranteed to work anymore. Since our users are using the store to create prejitted versions of the dependencies some Lambda functions that are using a different version then the resolved dependency in the store their performance will suffer as they silently stop using the prejitted version.

Keep in mind our users don't directly work with the store. Our tooling abstracts the store away.

Thanks @nguerrera for tagging me on the issue.

[nguerrera]

Thanks, @normj

Our tooling abstracts the store away.

What determines the version of the .NET Core SDK that use used to create the store artifacts?

silently stop using the prejitted version.

I'm asking above because it doesn't quite feel silent if there's some step to switch to 3.0 and generate a new store, where this could be documented.

The current behavior make the implementation quite complex and has caused quite a few issues:
#2160
#1682
#1380
#1072

Do you have a sense for how common it is to have a store in your system with multiple versions of the same package?

[normj]

We don't do anything special for choosing the SDK. We use the dotnet in the path to execute CLI commands and rely on dotnet's built in mechanisms for choosing the SDK.

Silent might not have been the right word. By silent I mean the user created in our case a Lambda layer which for .NET is a package store and expected all of their dependencies to be put in the store including multiple versions of the same assemblies. When the Lambda function is packaged our tooling calls the dotnet publish command passing in the manifests from the package stores. At that point they would assume the packaging of the Lambda function would not include any dependencies because the dependencies will be provided as part of the package store at runtime. Instead the dotnet publish command will find the particular version is not in the package store so instead included the non-prejitted dependency into the package folder.

They will see during the deployment the list of files being added to the package bundle so again silent was probably not the right word. That is assuming deployment is an interactive deployment and not a CI/CD deployment.

A switch when running dotnet store to be verbose about which package versions are not included in the store because a different version was chosen might make the situation more clear.

Sadly I don't have a sense of how common this is. The biggest benefit for the features is the prejitting since it dramatically cuts down cold starts. In those scenarios the proposal is completely fine.

I don't want to sound like I'm trying to hold changes back. I prefer you guys being able to push forward and clean things up and make things work great. What I need to understand is if you implement this change and a user does find themselves in the case of needing multiple versions in the package store what is the workaround to make that happen.

[nguerrera]

A switch when running dotnet store to be verbose about which package versions are not included in the store because a different version was chosen might make the situation more clear.

This would be great to have, unfortunately I don't know how to implement it in the planned simpler implementation I have in mind. Basically, I'd like to treat the manifest like any other project and stop doing heroics to restore each package in isolation. I don't think nuget returns us enough data to know what the delta is between every package's individual graph and the resolved graph is. cc @nkolev92

What I need to understand is if you implement this change and a user does find themselves in the case of needing multiple versions in the package store what is the workaround to make that happen.

At a lower level, the workaround would be to specify the other versions in a different manifest and run dotnet store multiple times. Could you support multiple manifests per "layer"?

[nguerrera]

Another workaround would be to add direct references where appropriate to lift things up to the version in your store.

Another thing you could do, I think, is to have all your projects have a project reference to your manifest project and not have individual package references to the things in the store. Instead let come transitively. It makes the manifest project like a meta package. The downside is that you're over-referencing things individual projects don't need, but that shouldn't be a big deal since the extra files will still be excluded from your publish output.

[nguerrera]

Another idea would be to warn on publish that we are deploying Foo vX as only vY is in the store. That is much easier to implement and I think it's less noisy.

[normj]

Are you saying dotnet publish would output that warning?

If dotnet publish did output warnings for version mismatch with passed in store manifests and I did the work in my tooling to support passing in a collection of project files to create the store I think that would satisfy our needs.

[nkolev92]

I don't think nuget returns us enough data to know what the delta is between every package's individual graph and the resolved graph is @nkolev92

Late response here, but that's correct, there's not enough data for that. Only the information for the selected packages will be contained in the assets file, so you could only recreate the overlapping parts of the graph.

[JunTaoLuo]

Any progress on this issue?

[nguerrera]

Yes, I have a pr up that should get merged tomorrow. It should fix the race conditions and allow tfm of 3.0 with store. But it's not the redesign I envisioned above nor does it change anything about storing multiple versions. I still hope to get that in, but it's quite big and I don't have an ETA. I believe nothing is blocked on it after the current pr goes in.

[nguerrera]

#3345

[JunTaoLuo]

I was curious because it might help with dotnet/aspnetcore#11186 which we will need for preview7. I'm not yet sure if this issue will block that but my hunch is that it shouldn't.