Improve auto-discovery of configuration assemblies in bundled mode

[0xced]

Neither the dependency context nor scanning DLL files on disk works with bundled (single-file) applications.

Given that the configuration assemblies have actually been loaded, using AppDomain.CurrentDomain.GetAssemblies() works for both bundled and non bundled applications.

[nblumhardt]

Thanks for tackling this, @0xced! 👏

Just a thought, since it's necessary in this mode to list assemblies specifically, could we get away from needing any scanning code at all (in 'bundled' mode), and just list all dependencies in the ReadFrom.Configuration() call?

E.g. for your example:

loggerConfiguration.ReadFrom.Configuration(
    context.Configuration,
    usingAssemblies: new[]
    {
        typeof(ConsoleLoggerConfigurationExtensions).Assembly,
        typeof(SeqLoggerConfigurationExtensions).Assembly
    })

[0xced]

You're absolutely right about explicitly passing the assemblies to the Configuration method, this is a much better (and simpler) approach!

But I think we can do even better and having the configuration assemblies automatically discovered even in bundled mode. I tried something in d8d162c which works fine on macOS but fails on Windows.

I implemented a proof-of-concept workaround in 3d2e5df which relies on an internal method of the Microsoft.NET.HostModel package. I'll rewrite it properly and once its done the Microsoft.NET.HostModel package dependency can be dropped altogether. So please don't merge this pull request as is, you could even turn it into a draft pull request for good measure.

Ideally, this logic should be implemented in the Microsoft.Extensions.DependencyModel project directly. I'll probably try to submit a pull request there so that eventually using DependencyContext.Default will work properly for both bundled and non bundled applications. That probably won't happen before .NET 7 or more realistically .NET 8, though.

[0xced]

I have rewritten the code (in 11bc6de) to read the bundled deps.json without taking a dependency on Microsoft.NET.HostModel, which as stated by Vitek Karas is not intended as a public API.

The hardest part was to write the SingleFileApp test in a safe way if run concurrently across different target frameworks by not sharing the obj directories when compiling the TestSingleFileApp project.

This pull request is now ready for review.

[nblumhardt]

Hi @0xced - thanks for the follow-up!

I'd be a little anxious to take this kind of dependency on the (undocumented?) bundle manifest and (loosely documented but possibly changeable?) .deps file formats; my guess is that the Microsoft.NET.HostModel API is not-for-public-consumption because the underlying infrastructure is subject to change 🤔

Still does make a nice job of addressing a tricky scenario, though - I might need to give it a bit more thought. @serilog/reviewers-configuration any sense of what would be best, here?

[0xced]

Toying around with single file apps and the deps.json file was a fun and very educative experiment. I learned a ton of things about single file apps by reading the design documents and the CoreCLR source code. But eventually it should really be implemented at the Microsoft.Extensions.DependencyModel level. I have even created a repository and written technical notes and thoughts in the README: https://github.com/0xced/SingleFileAppDependencyContext I took some code written for Serilog.Settings.Configuration (in this very pull request) and pushed the experiment a bit further. I hope this will serve as a good basis for discussing a stable implementation with the .NET Runtime team.

In the end the best solution to avoid listing assemblies in the configuration file is probably what you proposed in your initial reply, i.e. explicitly passing a list of assemblies to the Configuration method. Simple, straightforward!

Would you be interested in a pull request to implement this new Configuration overload? I guess it will be more work to update the documentation than to write the code itself. 😁

[skomis-mm]

Great work, @0xced 👍

IMO the risk of breaking changes in the bundle structure should be low.
Design of the bundle marker is public. Also, this comment says it can be used from the source code 🤔

[nblumhardt]

Okay, great to know! 😎

I'm not well-placed to review this currently, but gets a 👍 from me in case @skomis-mm or another maintainer is able to dig in deeply enough.

[0xced]

Closing this pull request in favor of #310.

Leaving https://github.com/0xced/SingleFileAppDependencyContext as a record of the work I did that should eventually be part of Microsoft.Extensions.DependencyModel.