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

Build a .NET Core Cli Tool #19

Closed
stephtr opened this issue Jun 2, 2018 · 19 comments

Comments

Projects
None yet
2 participants
@stephtr
Copy link
Contributor

commented Jun 2, 2018

It would be nice if this package could be published as a .NET Core Cli Tool.
An advantage would be, that one could simply add <DotNetCliToolReference Include="TypeGen" Version="1.6.0" /> to the *.csproj file and after restoring run dotnet TypeGen. Global install of the tool would then also be supported by dotnet tool install -g TypeGen.
Since there is already a TypeGen Cli it maybe wouldn't need much work.

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 4, 2018

This is a good idea and indeed sounds like maybe not that much work to do. I'll read up on how to do this and update here when I have a solution. Thanks for posting!

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 8, 2018

I've created a NuGet package to use with DotNet CLI: TypeGen.DotNetCli. You should specify it in DotNetCliToolReference (TypeGen.DotNetCli, version 1.6.1). I've checked it locally, but let me know if there are any issues.

@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2018

For basic setups it works fine. The only real issue is that within my project I'm using the ApiControllerAttribute (new to ASP.NET Core 2.1) on a controller. If I run the Cli on that project, it fails at GetCustomAttribute with:

GENERIC ERROR: Could not load type 'Microsoft.AspNetCore.Mvc.ApiControllerAttribute' from assembly 'Microsoft.AspNetCore.Mvc.Core, Version=1.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'.
   at System.ModuleHandle.ResolveType(RuntimeModule module, Int32 typeToken, IntPtr* typeInstArgs, Int32 typeInstCount, IntPtr* methodInstArgs, Int32 methodInstCount, ObjectHandleOnStack type)
   at System.ModuleHandle.ResolveTypeHandleInternal(RuntimeModule module, Int32 typeToken, RuntimeTypeHandle[] typeInstantiationContext, RuntimeTypeHandle[] methodInstantiationContext)
   at System.Reflection.RuntimeModule.ResolveType(Int32 metadataToken, Type[] genericTypeArguments, Type[] genericMethodArguments)
   at System.Reflection.CustomAttribute.FilterCustomAttributeRecord(CustomAttributeRecord caRecord, MetadataImport scope, Assembly& lastAptcaOkAssembly, RuntimeModule decoratedModule, MetadataToken decoratedToken, RuntimeType attributeFilterType, Boolean mustBeInheritable, Object[] attributes, IList derivedAttributes, RuntimeType& attributeType, IRuntimeMethodInfo& ctor, Boolean& ctorHasParameters, Boolean& isVarArg)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeModule decoratedModule, Int32 decoratedMetadataToken, Int32 pcaCount, RuntimeType attributeFilterType, Boolean mustBeInheritable, IList derivedAttributes, Boolean isDecoratedTargetSecurityTransparent)
   at System.Reflection.CustomAttribute.GetCustomAttributes(RuntimeType type, RuntimeType caType, Boolean inherit)
   at System.Attribute.GetCustomAttributes(MemberInfo element, Type type, Boolean inherit)
   at System.Attribute.GetCustomAttribute(MemberInfo element, Type attributeType, Boolean inherit)
   at System.Reflection.CustomAttributeExtensions.GetCustomAttribute[T](MemberInfo element)
   at TypeGen.Core.Extensions.TypeExtensions.HasExportAttribute(Type type)
   at System.Linq.Utilities.<>c__DisplayClass1_0`1.<CombinePredicates>b__0(TSource x)
   at System.Linq.Enumerable.WhereArrayIterator`1.MoveNext()
   at TypeGen.Core.Generator.<>c__DisplayClass11_0.<Generate>b__0()
   at TypeGen.Core.Generator.ExecuteWithTypeContextLogging(Action action)
   at TypeGen.Core.Generator.Generate(Assembly assembly)
   at TypeGen.Cli.Program.<>c__DisplayClass9_0.<Generate>b__0(IEnumerable`1 acc, Assembly assembly)
   at System.Linq.Enumerable.Aggregate[TSource,TAccumulate](IEnumerable`1 source, TAccumulate seed, Func`3 func)
   at TypeGen.Cli.Program.Generate(String projectFolder, String configPath, Boolean verbose)
   at TypeGen.Cli.Program.Main(String[] args)

If I run TypeGen programmatically from within my project it works just fine, but it also isn't working with the regular Cli. Maybe that's related to MVC Core being included in the Microsoft.AspNetCore.App semipackage, which is shipped within C:\Program Files\dotnet\shared\. Even though it just would be hiding symptoms, it would be possible to wrap the content of the HasExportAttribute method with a try{[…]}catch(TypeLoadException){Console.WriteLine('...');return false;}

Additionally I would have some ideas for improvements:

  • It would be nice if the Core Cli command would automatically run dotnet build and maybe even automatically find the assemblies necessary.
  • By default the Core Cli's ProjectFolder argument should default to . (such that one can simply run dotnet typegen instead of dotnet typegen .
@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2018

The issue with ApiControllerAttribute should get resolved if you specify C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.1.0 in externalAssemblyPaths config parameter (in tgconfig.json). This would be a temporary solution. I'll try to find a nice way to search in C:\Program Files\dotnet\shared by default, so that you don't need to specify it by hand.

Thanks for ideas for improvements! Below are my first thoughts:

I'll think about adding dotnet build. My first thought is that I'd like to keep it as 'single-responsibility' as possible, and since you don't always need to build the project when using TypeGen, I'd leave it as is. E.g. consider a scenario when you build your project, then use some other tool and then TypeGen - in this case your project would be built twice.

What do you mean by finding the necessary assemblies? If you mean the project's assembly, it should be found automatically (i.e. without adding it to the assemblies parameter in tgconfig.json). Could you specify which assemblies you have in mind?

ProjectFolder in TypeGen CLI doesn't have a default value by design - if someone is using TypeGen for the first time and just types typegen, I want the general help to be displayed, so that you know what you can do with the CLI and how to use it. Do you have some particular application in mind in which you would benefit from having typegen instead of typegen . (apart from having a shorter command to write)?

@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 12, 2018

I already tried that, even with externalAssemblyPaths added it still resolves it to C:\Users\<username>\.nuget\packages\microsoft.aspnetcore.mvc.core and takes the lowest version in there, but maybe I'll take a look on it on the weekend.

I'm not quite sure without looking it up but I think tools like the entity framework (for example dotnet ef database update) automatically build the code.

What do you mean by finding the necessary assemblies? If you mean the project's assembly, it should be found automatically

Ah thanks, I was a little bit confused because it didn't automatically resolve the ProjectReferences and I therefore added both assemblies to the assemblies property, but I just realized that adding the second project's path to externalAssemblyPaths is enough.

Do you have some particular application in mind in which you would benefit from having typegen instead of typegen . (apart from having a shorter command to write)?

No, it was mainly for semantic reasons: If I install it as a DotNetCliToolReference, why should I supply any path other than the project one's? If you want dotnet typegen to output the help, I would have slightly prefered dotnet typegen build over dotnet typegen ., but that's just a small nearly irrelevant design decision.

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 13, 2018

About resolving ApiControllerAttribute, I think the problem is that when resolving missing assemblies, TypeGen CLI doesn't check assembly versions - it only searches by assembly name, so it takes just any assembly that fits the name. The fix would be to add assembly version checking in TypeGen.Cli.Business.AssemblyResolver ResolveFromPaths method (instead of just loading the assembly, check the version first). Feel free to have a look at it if you fancy, otherwise I'll get it fixed probably sometime tomorrow.

About automatic building, maybe you're right - I haven't dug into dotnet CLI that much. If that's the convention, I'll add it. I'll check other tools first to see how they do it.

About renaming typegen . to typegen, I think I see what you mean. I haven't really used dotnet CLI before, but what you're suggesting makes sense. I'll also see other tools to see what convention are they using.

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 14, 2018

I've added assembly version checking and made a NuGet package that you can test locally - please tell me if it works. If it does, I'll make a release. This is not a dotnet CLI version, so you can check it with package manager console or just using TypeGen.ps1 that's in the package content.

@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2018

In principle it is working fine, except that generation of types now takes 4 minutes instead of 4 seconds, I'm just trying to find the cause.
Update: It seems like the issue is that it fails to resolve System.Runtime (which would be located in C:\Program Files\dotnet\shared\Microsoft.NETCore.App\2.1.0), so the system keeps calling AssemblyResolve, where each run to look in the nuget package folder takes two seconds.
But even if I feed it the correct location, calling Assembly.LoadFrom fails with:

Exception thrown: 'System.IO.FileLoadException' in System.Private.CoreLib.dll: 'Could not load file or assembly 'System.Runtime, Version=4.2.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'.'
@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2018

I noticed three points, which in sum would solve the problem:

  • C:\Program Files\dotnet\shared should be automatically detected, such that users don't have to add it as externalAssemblyPaths.
  • To me it seems like Assembly.LoadFrom could be replaced by Assembly.LoadFile, which would resolve the System.IO.FileLoadException mentioned above.
  • TypeGen currently supports metapackages only by recursively searching whole directories. If TypeGen won't actively support them, it should recursively look in C:\Program Files\dotnet\shared BEFORE recursively searching the nuget directory, since the latter one takes quite some time.

jburzynski added a commit that referenced this issue Jun 15, 2018

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 15, 2018

I made a new NuGet package with some changes:

  • all of your suggestions from the previous post (dotnet\shared folder, searching and Assembly.LoadFile)
  • added search of the 'global' NuGet fallback folder by default if exists (C:\Program Files\dotnet\sdk\NuGetFallbackFolder). May help with some other dependency issues.
@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2018

Thanks, for me it now is running perfectly!

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 15, 2018

Great to hear that! I'll try to make a release this weekend

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 18, 2018

I just made a release of 1.6.2. Apart from all the fixes, there is 1 change in the DotNet CLI version: you don't need to specify ProjectFolder. So you can type this: dotnet typegen which will generate files for current project. You can still specify a ProjectFolder (e.g. dotnet typegen myproject), but if you don't, it will default to . (current folder/project).

I'll also look at how/whether to do automatic building, but this may be included in the next release. You can check changelog to see what's being changed in each version.

Let me know if it works!

@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2018

Wow, that's fine!

While using it I noticed that it would be quite useful to have an option which would add an index.ts file,
which re-exports all generated classes, interfaces and enums. If you are interested in it, I could try to submit a PR in June or July.

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 19, 2018

Yes, I think that could be useful. Thanks for being interested in implementing it.

One thing I'm thinking about is that it should probably be opt-in, i.e. disabled by default, so that when you do a version update it doesn't do something you haven't asked for. Unless you have other idea - it's adjustable. The place to add the option would be TypeGen.Cli.Models.TgConfigand TypeGen.Core.GeneratorOptions. The two 'main' classes are TypeGen.Core.Generator and TypeGen.Cli.Program. All other things you should probably be able to see in the code.

Thanks again for having interest in making an upgrade!

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 21, 2018

It looks like things regarding .NET Core CLI tool have been resolved, so I'm closing this issue. Any new things will go to separate issues.

@jburzynski jburzynski closed this Jun 21, 2018

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 27, 2018

update on .NET Core CLI tool: there has been a change in syntax in 1.6.3, you can check out documentation.

@stephtr

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2018

Thanks for the information!
Just for information: ASP.NET Core 2.2 will probably contain a tool, which could contain the features of TypeGen (for details see the "API client generation (C# & TypeScript)" section in the roadmap). I'm a bit sorry that I dragged you into .NET Core Cli Tooling.

@jburzynski

This comment has been minimized.

Copy link
Owner

commented Jun 28, 2018

Thanks for the info! Looks interesting. Maybe TypeGen will still be useful though in some scenarios, we'll see.

Building a .NET Core CLI tool was not a big problem, so no worries. Most of this thread refers to an issue that would need to be resolved anyway (finding dlls). Converting TypeGen to dotnet CLI was not very problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.