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

Improve task DLL load behavior #1312

Closed
rainersigwald opened this issue Nov 4, 2016 · 32 comments
Closed

Improve task DLL load behavior #1312

rainersigwald opened this issue Nov 4, 2016 · 32 comments
Labels
needs-design Requires discussion with the dev team before attempting a fix. triaged
Milestone

Comments

@rainersigwald
Copy link
Member

This is a long-term feedback-desired issue.

See #1307 (comment), from @ericstj:

@tmat and I were discussing the other day that it probably makes more sense for MSBuild to hook AssemblyResolve. That way tasks don't have to worry about binding redirects. You'd still hit issues with ordering where you encounter a task early that loads an older version of a library than one needed by a later task, but at least it would handle a common case where folks have conflicts within a task. Right now tasks themselves have to deal with this. Other ideas: If you have a complete view of all task directories you could unify up front for all possible assemblies. If you don't have a complete view you could do a look when you're about to instantiate a task to determine if it would have a conflict and open a new app domain if it would.

@rainersigwald rainersigwald added the needs-design Requires discussion with the dev team before attempting a fix. label Nov 4, 2016
@mwpowellhtx
Copy link

Would the .deps.json work?

@rainersigwald
Copy link
Member Author

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 10, 2019

@rainersigwald When does the .deps.json file land in the folder? Seems like it only lands in the bin folder, question is: when. That's pretty critical. At the initial stages of the build? In sufficient timing for a pre-build code generation to take place? That is, if we were to parse the file and walk the dependencies for internal use, ensure that the flattened dependencies all land in the binary folder just prior to code generation taking place.

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 10, 2019

@rainersigwald Then the question would be, for those of us on .NET Standard 2 or .NET Core 2, that is. Rolling something custom along those lines.

... will have to target netcoreapp3.0 to use these APIs.

Or roll our own approach.

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 10, 2019

@rainersigwald The corehost_resolve_component_dependencies and corehost_set_error_writer callbacks seem to be the nerve center for the whole resolution procedure.

Is there documentation for the hostpolicy.dll?

More importantly, short of rolling our own v2 variations, these imported functions are accessible to netstandard2.0 targeted assemblies.

@rainersigwald
Copy link
Member Author

@mwpowellhtx I don't think I see how your questions relate to this issue. Can you clarify?

@mwpowellhtx
Copy link

@rainersigwald I just followed the links in the MD. I see a Resolver that is "implemented", and my question is whether Host Policy is a v3 SDK thing, or whether that is exposed today in the v2 SDK. i.e. that we could back port that.

@rainersigwald
Copy link
Member Author

@vitek-karas, do you know of docs on that?

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 12, 2019

@rainersigwald @vitek-karas To be clear, from the MD here, and just followed the links. From there I did my due diligence as far as I could dig into the source code and suss out the invocations. @rainersigwald And please do not tell me TL;DL, that is just insert appropriate expletive lazy, IMO.

@vitek-karas
Copy link
Member

AssemblyDependencyResolver is .NET Core 3.0 only. It's not in netstandard (yet... I guess).
We don't have docs on hostpolicy APIs. While technically they are public, I would be careful taking dependencies on them as we think we are OK to change them between major releases.

I must admit I don't understand the other part of this discussion about when the .deps.json drops during the build. My understanding of this issues is that it's about loading tasks - which ideally would come with their own .deps.json files describing the dependencies of the tasks. The proposal above is about how to load tasks such that they can have isolated subtrees of dependencies. It has no link to .deps.json potentially produced during running those tasks.

.deps.json is a .NET Core only concept, so if we would be to improve the task loading using that, such an improvement would be .NET Core only in nature.

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 12, 2019

@vitek-karas My concern here would be that the Resolver is taking a dependency on hostpolicy; this after reviewing the code and discovering the DLL invocations. Are these available in the v2 SDK if we wanted to expose them for v2 work?

@rainersigwald Also, to clarify, we are having this discussion because of precisely this issue. Dependencies are failing to resolve for build tasking. Specifically I have a Code Generation asset which I want to run internal to a project, that itself carries half a dozen mission critical unresolved dependencies. So this is a show stopper until we can put some sort of workaround, or longer range solution, into production.

Our objective is to ensure that the dependencies have been resolved, and/or copied into the bin folder, then potentially packaging a fat package for internal delivery. After that point, we are 90% confident that we would see a successful CG take place. Leading up to that point, the CG bits themselves, and contributing bits in between, are all working and have been unit and integration tested, so this is a moment of truth for us bridging this gap.

@livarcocc
Copy link
Contributor

I don't believe depending on deps.json is the right path forward here. It is a runtime asset and not a build time one. What we should do is figure out what sorts of inputs you would need to hook your tasks for those inputs, if you are trying to find the right types, files, etc in your task. If you task itself will depend on varying degrees of assemblies, then Vitek proposal is the closer we have right now, but it is not implemented and we haven't taken that forward yet. I am sorry about that.

@mwpowellhtx
Copy link

@livarcocc Begging your pardon, but for us, the runtime happens to be a dotnet CLI tooling whose scope is build time. So, yes, we do need to resolve these dependencies during the build. If you'd like to discuss further or in more depth, we should establish an NDA.

@mwpowellhtx
Copy link

@livarcocc @vitek-karas I would also like to clarify the dependency System.Runtime.Loader is taking on hostpolicy.

We don't have docs on hostpolicy APIs. While technically they are public, I would be careful taking dependencies on them as we think we are OK to change them between major releases.

Specifically, the degree to which they may be exposed to v2 SDK. That is, whether they are; although at this point, we are inferring possibly they are only exposed to v3 SDK.

Absent or short of that, what alternative paths or workarounds we can pursue on our end.

@rainersigwald
Copy link
Member Author

@mwpowellhtx Since MSBuild will have no built-in support for .deps.json in SDK 2.x or 3.0, I think you should consider moving your codepath with complex dependencies into its own process and just invoking that during the build.

@vitek-karas
Copy link
Member

I think we probably don't understand what you want to use the .deps.json for.

Let's assume that your task is called CustomCodeGen.dll, and your application is App.dll. There are potentially two .deps.json files we can discuss:

  • CustomCodeGen.deps.json - this describes the dependencies of the custom task itself. By its nature this cannot have any ties to the App.dll since the task has to exist before the App.dll ever built. This file can be consumed by the build (as per the proposal mentioned above) to provide better handling of dependencies of tasks. It is in theory possible to implement this completely inside the task itself, although it's not exactly easy.
  • App.deps.json - this file is produced by the build and/or publish of the App if it is targeting netcoreapp framework. It describes the dependencies of the App.dll alone.

AssemblyDependencyResolver is meant to be used to resolve dependencies of the currently running app (or its plugins). So if it's used by CustomCodeGen (or rather msbuild) then it should be used on CustomCodeGen.deps.json.

If you want to "inspect" "a random" .deps.json there's the Microsoft.Extensions.DependencyModel NuGet package which can read or write .deps.json files using a relatively easy to use OM.

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 12, 2019

@vitek-karas My analysis thus far has us focused on the generators themselves, i.e. plug-ins to the Code Generation Engine.

These are failing to load into the Code Generator assembly resolution, which I think, with 97% certainty, has to do with the failure to resolve the generators dependencies. Currently focused on a fat packaging approach for internal PrivateAssets="all" delivery during subsequent build configurations during our build pipeline.

If you want to "inspect" "a random" .deps.json there's the Microsoft.Extensions.DependencyModel NuGet package which can read or write .deps.json files using a relatively easy to use OM.

Yes, we are trying to use the CompositeCompilationAssemblyResolver these during assembly resolution, but this is not working for the dotnet CLI tooling.

Corollary with this approach, as we understand it, this may be similar to how NET Core / Standard are being built, but we did not receive adequate clues when we received feedback via Gitter channels in order to independently verify this. I apologize I do not have the comment link handy; through one of the pertinent MSBuild or CLI channels, I think.

As long as our tooling can load the generator(s), including the generator dependency(ies), I think we have a path forward. Trying to at least verify that in the shortest distance possible. At the moment, using a custom build task and manually copying from the NuGet restored \path\to\packages\**\dependencies\* assets. Downstream from that, we look at possibly a more in depth analysis of .deps.json.

I should clarify here, restored and not resolved, because actual resolution is falling over at the moment.

I do not think we are concerned about the target assembly, i.e. where the code generation will actually land. That is not the issue for us.

Hopefully this helps improve the comprehension.

@vitek-karas
Copy link
Member

Thanks a lot for your explanation. In this case you can at least try the AssemblyDependencyResolver. You would need to use 3.0 SDK (since it runs on 3.0 .NET Core) and you can try to hook up AssemblyLoadContext.Default.Resolving event and in it use the AssemblyDependencyResolver. At the very least this would tell you which dependencies are "missing" (those will hit the event handler) and hopefully you'll be able to resolve them using the AssemblyDependencyResolver.

It can also be the case that your generator has a dependency which something else already loaded and there's a version mismatch. In this case there's unfortunately not a simple solution. In theory you could be able to isolate your generator into its own AssemblyLoadContext and that way you get full control over your dependencies (and it will let you load a second version of the same assembly side-by-side). A sample of such an app is here: AppWithPlugin, but it's an app loading plugins - in your case you would have to make your task "load itself" as a plugin - it's doable, but requires splitting the task into two assemblies (one acts as the facade, and loads the other into its own ALC).

But if it's only about not finding the right files, then the first approach should work - and that is pretty simple - look at the AppWithPlugins it uses AssemblyDependencyResolver for a something similar (not exactly the same though).

That said - if you build your generator as netcoreapp3.0 it should include all its managed dependencies in the output and the .deps.json is basically just pointing to the directory. The real value of .deps.json comes with portable apps/plugins where they carry different assets for different platforms - which doesn't sound like your case.

@mwpowellhtx
Copy link

@vitek-karas This is the rub, at the moment this is part of the gap we must bridge. We need a netstandard2.0 capable approach.

@vitek-karas
Copy link
Member

@mwpowellhtx In that case:

  • If your code generator is basically pure IL (no native code, and/or you're going to run it only on one platform), I would go with the simple approach of copying all dependencies into the output. Then all you need is potentially to add an event handler which will look for the assemblies in the generator folder.
  • If you truly need platform specific dependencies, then go with .deps.json and use the DependencyModel - it's not exactly simple (the AssemblyDependencyResolver is much easier to use), but in the end it does provide similar functionality.

@mwpowellhtx
Copy link

@vitek-karas We are having this conversation because System.Runtime.Loader is failing to resolve those dependencies, at least in its current 4.3.0 form. See prior mentioned links, and please look at that so we're on the same page not talking past one another, if you please.

Sounds like our near term path forward is in point of fact to dissect the .deps.json and do some manual, or at least custom task-based, resolving of the dependencies.

@vitek-karas
Copy link
Member

@mwpowellhtx Sorry - I missed that part of your comment - my bad. It's hard to tell why it's failing... I can only guess:

  • If you're using a simple dotnet build output as your generator that will not copy NuGet dependencies to its output (with 2.0 SDK) and instead it relies on .runtimeconfig.dev.json to provide additional probing paths which point to NuGet cache folders. This will obviously not work too well when you're running in the context of the Roslyn compiler which does not have the necessary additional probing paths.

I obviously don't know how exactly you build your generator component... when I try a simple classlib project with some NuGet dependencies, running dotnet publish on it seems to copy all the dependencies into the output - so that might help with the above mentioned problem with probing paths.

Unfortunately we don't have good enough tracing available yet to be able to answer with relative ease exactly why it's failing. You should be getting at least an exception of some kind which should indicate which assembly is failing to resolve, but I personally don't have much experience with code generators in Roslyn.

I just realized I didn't answer your question about the hostpolicy APIs used by the AssemblyDependencyResolver - those are unfortunately only available in .NET Core 3.0 (they are new there).

@mwpowellhtx
Copy link

@vitek-karas No worries, I appreciate the clarification. So if we did anything with the .deps.json from a historical perspective, we would be considering contemporary API scaffold of sorts, that is, targeting the netstandard2.0, IL if you prefer, for purposes of rolling our own. That is if we found it necessary to peel that layer back at all.

We are at a point, we are copying the asset dependencies from their resolved package locations, and poised to create a fat package. After that try to subscribe to it and use it as our code generator, including its dependencies in the package. We will see how far we get in that approach.

Thanks again.

@mwpowellhtx
Copy link

@vitek-karas I should also mention, we do get an exception that the reference asset cannot be loaded, but that's all we get. There is no further detail, to my knowledge, no inner exception, etc, so it is not especially helpful. That is, was it because dependencies could not be loaded as well? Along these lines, as an educated guess-timate, at any rate.

@vitek-karas
Copy link
Member

Can you share the type of exception you get? I'm just curious... we're working on improving diagnostics around the assembly loader/binder and it would help to know which cases are problematic.

And yes - we know the diagnostics around this area right now is pretty bad, we're working on it (rather slowly unfortunately).

@mwpowellhtx
Copy link

@vitek-karas Sure, at the moment, along these lines:

5>Evaluating `dotnet cgr --response "obj\Debug\netstandard2.0\MyApplication.csproj.rsp"´ ...
5>Code.Generation.Roslyn.CodeGenerationDependencyException: Unable to load dependency `D:\Path\To\src\packages\mygenerators\1.0.0.16569\lib\netstandard2.0\MyGenerators.dll'.
5>Code.Generation.Roslyn.CodeGenerationDependencyException: Unable to load dependency `D:\Path\To\src\packages\mygenerators\1.0.0.16569\lib\netstandard2.0\MyGenerators.dll'. ---> System.ArgumentNullException: Value cannot be null.
5>Parameter name: other
5>   at Microsoft.Extensions.DependencyModel.DependencyContext.Merge(DependencyContext other)
5>   at Code.Generation.Roslyn.CompilationAssemblyResolverDependencyContext.AddDependency(String path, Assembly assembly)
5>   --- End of inner exception stack trace ---
5>   at Code.Generation.Roslyn.CompilationAssemblyResolverDependencyContext.AddDependency(String path, Assembly assembly)
5>   at Code.Generation.Roslyn.AssemblyReferenceServiceManager.TryRegisterMatchingAssembly(String candidateAssemblyPath, Assembly& assembly)
5>   at Code.Generation.Roslyn.AssemblyReferenceServiceManager.LoadAssembly(AssemblyName assemblyName)
5>   at Code.Generation.Roslyn.AttributeDataExtensionMethods.<GetCodeGeneratorTypeForAttribute>g__LoadGeneratorTypeFromAssembly|4_0[TSymbol](String fullTypeName, String assemblyName, <>c__DisplayClass4_0`1& )
5>   at Code.Generation.Roslyn.AttributeDataExtensionMethods.GetCodeGeneratorTypeForAttribute[TSymbol](TSymbol attributeType, LoadAssemblyCallback loader)
5>   at Code.Generation.Roslyn.AttributeDataExtensionMethods.<>c__DisplayClass3_0`1.<LoadCodeGenerators>b__1(AttributeData x)
5>   at System.Linq.Enumerable.SelectArrayIterator`2.MoveNext()
5>   at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.ToArray()
5>   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
5>   at Code.Generation.Roslyn.AssemblyTransformation.<>c__DisplayClass1_0.<<TransformAsync>g__GetTransformations|0>d.MoveNext()
5>   at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
5>   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
5>   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
5>   at Code.Generation.Roslyn.CompilationServiceManager.TransformCompiledCode[TTransformation](CSharpCompilation compilation, IProgress`1 progress, CancellationToken cancellationToken, TTransformation transformation, String sourceFilePath, ConfigureTransformationCallback`1 configure)
5>   at Code.Generation.Roslyn.CompilationServiceManager.FacilitateAssemblyCodeGeneration(CSharpCompilation compilation, IProgress`1 progress, CancellationToken cancellationToken)
5>   at Code.Generation.Roslyn.CompilationServiceManager.Generate(IProgress`1 progress, CancellationToken cancellationToken)
5>   at Code.Generation.Roslyn.ToolConsoleManager.OnGenerate(Int32 _)

In its current form, but I have been trying to wire out more informative messages in my working branch. However, I can say our Response File is trying to do the right thing.

@vitek-karas
Copy link
Member

I find this interesting:

System.ArgumentNullException: Value cannot be null.
5>Parameter name: other
5>   at Microsoft.Extensions.DependencyModel.DependencyContext.Merge(DependencyContext other)
5>   at Code.Generation.Roslyn.CompilationAssemblyResolverDependencyContext.AddDependency(String path, Assembly assembly)

If that is running the code you're pointing at, it should pretty much never happen... or it could be bug in Microsoft.Extensions.DependencyModel - in which case I would be VERY interested.

@mwpowellhtx
Copy link

@vitek-karas As I stated earlier, I am mid-stream focusing on repackaging our code generation generators in a fat-package, and evaluating if that works any better resolving references. Will let you know how that goes.

@mwpowellhtx
Copy link

mwpowellhtx commented Aug 17, 2019

@vitek-karas Took me a bit longer than I anticipated to circle around on this one, however, it is still not working. I fat-packaged my code generators, the response file indicates those references would be loaded, ostensibly in the correct order, from least dependent to more dependent, and so on.

The actual Response File my Code Generation facilitation generates is attached.

Insofar as the dependencies have been correctly fat packaged, they are there in the references correctly, this much I can tell you at this moment.

I need to review my code generation code re: the references in particular, but otherwise, I do not see how the assemblies could not be loaded. I will also add additional validation in my AddDependency code where assembly and (DependencyContext) other are concerned.

Edit: @vitek-karas The assets are indeed there, I validated my loaded Assembly is not null. Validation does fall over on the front side of invoking Merge, validating the attempted DependencyContext.Load(assembly):

3>Parameter name: assyDependencyContext
3>   at Validation.Requires.NotNull[T](T value, String parameterName)
3>   at Code.Generation.Roslyn.CompilationAssemblyResolverDependencyContext.AddDependency(String path, Assembly assembly)

I will double check I am not simply loading the top-level asset, but indeed all of the actual dependency references.

@livarcocc
Copy link
Contributor

We have no plans to implement this for Full Framework MSBuild. We are working on this feature for .NET Core MSBuild using AssemblyLoadContext.

@livarcocc livarcocc added this to the Backlog milestone Nov 4, 2019
@zachrybaker
Copy link

@rainersigwald
Copy link
Member Author

@zachrybaker We do not plan to change assembly loading behavior on .NET Framework. Further improving .NET Core/5.0+ load behavior is possible and tracked by #5037.

@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-design Requires discussion with the dev team before attempting a fix. triaged
Projects
None yet
Development

No branches or pull requests

6 participants