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

Supporting netcoreapp3.0 with McMaster.Extensions.Hosting.CommandLine #294

Closed
cwoolum opened this issue Oct 29, 2019 · 11 comments · Fixed by #336
Closed

Supporting netcoreapp3.0 with McMaster.Extensions.Hosting.CommandLine #294

cwoolum opened this issue Oct 29, 2019 · 11 comments · Fixed by #336

Comments

@cwoolum
Copy link

cwoolum commented Oct 29, 2019

I am trying to use this library with netcoreapp3.0. I get a failure whenever I try to launch the app.

'Method 'UseServiceProviderFactory' in type 'Microsoft.Extensions.Hosting.HostBuilder' from assembly 'Microsoft.Extensions.Hosting, Version=2.1.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60' does not have an implementation.'

I think this is because McMaster.Extensions.Hosting.CommandLine.csproj refers to [email protected].

My main method looks like

  private static async Task<int> Main(string[] args)
        {
            return await new HostBuilder()
                 .ConfigureAppConfiguration((hostContext, configApp) =>
                 {
                     var tokenProvider = new AzureServiceTokenProvider();
                     var kv = new KeyVaultClient(new KeyVaultClient.AuthenticationCallback(tokenProvider.KeyVaultTokenCallback));

                     configApp.SetBasePath(Directory.GetCurrentDirectory());
                     configApp.AddJsonFile("appsettings.json", optional: true);
                     configApp.AddJsonFile(
                         $"appsettings.{hostContext.HostingEnvironment.EnvironmentName}.json",
                         optional: true);
                     configApp.AddCommandLine(args);

                     configApp.AddAzureKeyVault(
                       $"https://{configApp.Build().GetValue<string>("Vault")}.vault.azure.net/",
                       kv,
                       new DefaultKeyVaultSecretManager());
                 })

               .ConfigureServices((context, services) =>
               {
                   var provider = services.BuildServiceProvider();                 

                   services.RegisterDbContext();
               })

           .RunCommandLineApplicationAsync<Program>(args);
        }
@cwoolum cwoolum changed the title Supporting netcoreapp3.0 Supporting netcoreapp3.0 Oct 29, 2019
@ghost
Copy link

ghost commented Oct 29, 2019

Im using this Library in a project that uses netcoreapp3.0 https://github.com/ppadial/netcore3-cli-sample and is working fine.

@cwoolum
Copy link
Author

cwoolum commented Oct 29, 2019

I think the issue is with using HostBuilder and RunCommandLineApplicationAsync which I'm not seeing in your sample app.

@cwoolum cwoolum changed the title Supporting netcoreapp3.0 Supporting netcoreapp3.0 with McMaster.Extensions.Hosting.CommandLine Oct 29, 2019
@ghost
Copy link

ghost commented Oct 29, 2019

Hey @cwoolum , yes, sorry.

I was taking a look to the test folder, it is a Hosting project that looks using netcoreapp3

<TargetFrameworks>netcoreapp3.0;netcoreapp2.2</TargetFrameworks>
and it's using the hostbuilder:

Reading the doc here (https://natemcmaster.github.io/CommandLineUtils/docs/advanced/generic-host.html) i've created an small app to verify it and looks working, this is the code:

using System;
using System.Threading.Tasks;
using McMaster.Extensions.CommandLineUtils;
using Microsoft.Extensions.Hosting;

namespace hostingdemocli
{
    class Program
    {
        static Task<int> Main(string[] args)
            => new HostBuilder()
            .RunCommandLineApplicationAsync<Program>(args);

        [Option]
        public int Port { get; } = 8080;

        private IHostingEnvironment _env;

        public Program(IHostingEnvironment env)
        {
            _env = env;
        }

        private void OnExecute()
        {
            Console.WriteLine($"Starting on port {Port}, env = {_env.EnvironmentName}");
        }
    }
}

This is the csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="McMaster.Extensions.CommandLineUtils" Version="2.4.2" />
    <PackageReference Include="Microsoft.Extensions.Hosting" Version="3.0.0" />
    <PackageReference Include="McMaster.Extensions.Hosting.CommandLine" Version="2.4.2" />
  </ItemGroup>
</Project>

Is it possible that your error came from an issue with another dependency in your project and HostBuilder? or a dependency injection issue?

@cwoolum
Copy link
Author

cwoolum commented Oct 29, 2019

Aaah, I see the problem. It doesn't work unless

<PackageReference Include="Microsoft.Extensions.Hosting" Version="3.0.0" />

is explicitly specified. I didn't have that because McMaster.Extensions.Hosting.CommandLine was bringing it in previously. I've added a PR that brings in the correct version depending on if you are NetStandard 2.0 or 2.1. I can add the package explicitly until it gets merged in.

@ghost
Copy link

ghost commented Oct 29, 2019

Cool :)

@natemcmaster natemcmaster added area-hosting bug good first issue This seems like a good issue if you're a new contributor. help wanted We would be willing to take a well-written PR to help fix this. labels Oct 30, 2019
@natemcmaster
Copy link
Owner

@cwoolum can you share a standalone repro? I'd like to know what is actually causing the issue. UseServiceProviderFactory isn't used anywhere in this code base, so I suspect there is actually something else in your dependency graph causing this issue.

@natemcmaster natemcmaster added more-info-needed Please provide more information. and removed bug good first issue This seems like a good issue if you're a new contributor. help wanted We would be willing to take a well-written PR to help fix this. labels Oct 30, 2019
@cwoolum
Copy link
Author

cwoolum commented Oct 30, 2019

I figured out what the issue is. Looking down my dependency tree, I have a reference to Microsoft.Extensions.Diagnostics.HealthChecks which refers to [email protected]. If I remove that reference everything works fine.

At compile time, it seems that there are no problems using the 3.0.0 version of abstractions with the 2.1.0 implementation but at runtime, I think there is some reflection happening in the HostBuilder that might be causing the issue.

Here is a repo with a repro: https://github.com/cwoolum/294-repro

@no-response no-response bot removed the more-info-needed Please provide more information. label Oct 30, 2019
@natemcmaster
Copy link
Owner

Thanks for the repro!

(*) - hoisted from 2.1.0 to 3.0.0

Here is the problem: dotnet/extensions#612. Microsoft.Extensions.Hosting.Abstractions made a breaking change between 2.1 and 3.0....they added a new method to an interface. NuGet is hoisting the .Abstractions package to a new version because of your dependency on M.E.Diagnostics.HealthChecks. [email protected] does not implement the new interface method added in dotnet/extensions#612, so .NET cannot use [email protected] as an implementation of the interface [email protected].

Solutions

  1. As proposed in Import the proper Microsoft.Extensions.Hosting lib for netstandard2.1 #296, upgrade command line utils to use hosting 3.0
  2. As commented above, .NET Core 3.0 users need to add an explicit reference to <PackageReference Include="Microsoft.Extensions.Hosting" Version="3.0.0" />
  3. Simplify CommandLineUtils's dependency to only rely on M.E.Hosting.Abstractions.

@natemcmaster
Copy link
Owner

I'm currently favoring solution (3) - see draft commit in 494bbc8. The advantage is that it maximizes compatibility. CommandLineUtils would not need to multi-target to support both 2.1 and 3.0. (It also fits with my philosophy on dependencies is that class libraries --- that is, class libraries should minimize dependencies and use the lowest version possible to maximize compatibility.) The disadvantage is that it's a packaging breaking change; anyone using CommandLineUtils doesn't explicitly depend on Microsoft.Extensions.Hosting would need to add a new package reference.

Thoughts?

@cwoolum
Copy link
Author

cwoolum commented Oct 30, 2019

I like option 3 but don't like introducing a breaking change unless this package version is going to jump to 3.x..

Would it be better to go with option 1 for now and then implement option 3 when a new major version this package is released so consumers aren't surprised? I can also change the framework further to lessen any side-effects. Instead of netstandard2.2, I can target netcoreapp3.0.

@natemcmaster
Copy link
Owner

I’m hesitant on option 1 because it seems odd to me that the version of the dependency should vary based on target framework. Microsoft.Extensions.Hosting and CommandLineUtils are both netstandard2.0 libraries. If we go with option 1, netcoreapp3.0 users are forced into a higher version of Hosting, even if they were trying to use Hosting 2.1 on netcorapp3.0 (a perfectly valid use case, btw.)

I think I’d like to wait on this one (so, do option 2), and revisit this for the next major version of CommandLineUtils.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants