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

.NET Core support #71

Closed
picrap opened this issue Oct 13, 2016 · 68 comments
Closed

.NET Core support #71

picrap opened this issue Oct 13, 2016 · 68 comments

Comments

@picrap
Copy link
Member

picrap commented Oct 13, 2016

Because right now, it's no 😢

@AlexanderButs
Copy link

AlexanderButs commented Oct 13, 2016

Take into account project.json stuff - right now to make it working you should add post build step to project.json (because there is no msbuild for .NET Core and your build target is useless). But MS is planning to move project.json and xproj system back to msbuild and csproj. So read info about that first of all.

@picrap
Copy link
Member Author

picrap commented Nov 8, 2016

@picrap
Copy link
Member Author

picrap commented Nov 25, 2016

The best option is to wait for .NET Standard 2.0, which is supposed to be supported by all recent frameworks.

@picrap picrap added the on hold label Nov 25, 2016
@DBCI
Copy link

DBCI commented Dec 1, 2016

Hello Pascal,

First, I want to say that MrAdvice is absolutely wonderful! However, I need it to work with .NET Core projects and as such I've tried to add the MrAdvice target to my project and got the following result:

warning : Can't load 'MrAdvice, Version=2.0.0.0, Culture=neutral, PublicKeyToken=c0e7e6eab6f293d8'
error : Internal error during Types import: System.NullReferenceException: Object reference not set to an instance of an object.
error : at ArxOne.MrAdvice.Weaver.AspectWeaver.CreateWeavingContext(ModuleDefMD moduleDefinition)
error : at ArxOne.MrAdvice.Weaver.AspectWeaver.Weave(ModuleDefMD moduleDefinition)

This appears at the build output right after the project properly builds.
I'm currently at a loss as to how I can resolve this, any ideas or comments would be great! :)

Again, wonderful product!

Thanks,
DB

@picrap
Copy link
Member Author

picrap commented Dec 1, 2016

Hi DB,
Yes I currently have the same problem and planned to work on it today. I'll let you know here when it is fixed.

And of course, I'm glad that you like Mr. Advice 👍

@picrap
Copy link
Member Author

picrap commented Dec 2, 2016

I've released a version 2.4.1-test1 which probably won't solve your problem (it solved mine, but I'm not sure they are the same).
If it still fails, could you provide me a simple project with the problem?

@DBCI
Copy link

DBCI commented Dec 2, 2016

Hello Pascal, and thanks for the speedy reply! :)

My issue is that I am actually trying to use MrAdvice (NuGet package 2.4.1-test1; just updated to it) in a .NET Core Console app project (*.xproj project.json files).

I took exactly the same code I've created when testing MrAdvice on my initial project (.NET Framework 4.6.1) and your NuGet package doesn't even modify the .NET Core project files to include the Weaver.

When I "forcibly" added the Weaver to the .NET Core project, that resulted in the error I showed you before. I will attempt to replicate this with your newest version and see what happens... stay tuned... ;)

@picrap
Copy link
Member Author

picrap commented Dec 2, 2016

Don't hurry, I think I have the exact same problem as you right now, so wait until a -test2 version is available.

@DBCI
Copy link

DBCI commented Dec 2, 2016

OK, thanks...

By the way, have you started to adjust everything to work with the Mono.CeCIL .NET Core library version? Or do you have any other recommended CIL Re-writing Library you prefer working with?

@picrap
Copy link
Member Author

picrap commented Dec 2, 2016

My problem was just a debug parameter, so nothing to fix and no -test2 needed right now, so you can probably use the -test1.
Regarding Cecil, I left it. Instead I use dnlib.

@DBCI
Copy link

DBCI commented Dec 2, 2016

Does dnlib support .NET Core?

@picrap
Copy link
Member Author

picrap commented Dec 2, 2016

Running from a .NET Core application? I have no idea...

@DBCI
Copy link

DBCI commented Dec 2, 2016

Thanks for the advice! 👍 I'll check dnlib out and see what I can come up with.

@DBCI
Copy link

DBCI commented Dec 6, 2016

Hi Pascal... my journey continues 😉

This is what VS2015 reports when I build my DNC test project:

1>------ Rebuild All started: Project: AOPDNCTest, Configuration: Debug Any CPU ------
1>  C:\Program Files\dotnet\dotnet.exe build "C:\Users\dbarak\Source\Testing\AOPDNCTest\src\AOPDNCTest" --configuration Debug --no-dependencies --no-incremental
1>  Project AOPDNCTest (.NETCoreApp,Version=v1.0) will be compiled because project is not safe for incremental compilation. Use --build-profile flag for more information.
1>  Compiling AOPDNCTest for .NETCoreApp,Version=v1.0
1>  Compilation succeeded.
1>      0 Warning(s)
1>      0 Error(s)
1>  Time elapsed 00:00:01.3288220
1>   (The compilation time can be improved. Run "dotnet build --build-profile" for more information)
1>  MrAdvice 2.4.0 weaved module 'AOPDNCTest, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' (targeting framework ) in 202ms
========== Rebuild All: 1 succeeded, 0 failed, 0 skipped ==========

And this happens as soon as I try to run the program:

{System.ArgumentException: Type passed in must be derived from System.Attribute or System.Attribute itself.
   at System.Attribute.GetCustomAttributes(MemberInfo element, Type type, Boolean inherit)
   at ArxOne.MrAdvice.PlatformUtility.GetAttributes[TAttribute](Type provider)
   at System.Linq.Enumerable.<SelectManyIterator>d__157`2.MoveNext()
   at System.Linq.Enumerable.UnionIterator`1.MoveNext()
   at System.Linq.Enumerable.SelectEnumerableIterator`2.MoveNext()
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source, Int32& length)
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at System.Linq.Enumerable.ToArray[TSource](IEnumerable`1 source)
   at ArxOne.MrAdvice.Invocation.GetAllAdvices[TAdvice](MethodBase targetMethod, Tuple`2& relatedPropertyInfo)
   at ArxOne.MrAdvice.Invocation.GetAdvices[TAdvice](MethodBase targetMethod, Tuple`2& relatedPropertyInfo)
   at ArxOne.MrAdvice.Invocation.CreateAspectInfo(MethodBase method, RuntimeMethodHandle methodHandle, MethodInfo innerMethod, RuntimeMethodHandle innerMethodHandle, Boolean abstractedTarget)
   at ArxOne.MrAdvice.Invocation.GetAspectInfo(MethodBase method, RuntimeMethodHandle methodHandle, MethodBase innerMethod, RuntimeMethodHandle innerMethodHandle, Boolean abstractedTarget, Type[] genericArguments)
   at ArxOne.MrAdvice.Invocation.ProceedAdvice(Object target, Object[] parameters, RuntimeMethodHandle methodHandle, RuntimeMethodHandle innerMethodHandle, RuntimeTypeHandle typeHandle, Boolean abstractedTarget, Type[] genericArguments)
   at ArxOne.MrAdvice.⚡Invocation.ProceedAspect(Object , RuntimeMethodHandle , RuntimeMethodHandle )
   at AOPDNCTest.Computer.Compute()
   at AOPDNCTest.Program.Main(String[] args)}

Any ideas what's going on?

@picrap
Copy link
Member Author

picrap commented Dec 7, 2016

Probably: this means that I misused the .GetCustomAttributes() method. This should be fixed in version 2.4.3.

@DBCI
Copy link

DBCI commented Dec 7, 2016

I'll try to update to v2.4.3 and see how it goes... Thanks! 😄

What do you know... it works! Whatever you did in 2.4.3, properly activates the advice now!

I don't think your v2.4.3 nuget package modifies the project to add the weave target, but this is a start! 😂

@picrap
Copy link
Member Author

picrap commented Dec 7, 2016

What package do you use? Net40 or NetStandard16?
The problem with dotnet build is that I have no idea on how to work with it, since I don't know what NuGet can change to .NET Core projects (xproj, is that right?)

@DBCI
Copy link

DBCI commented Dec 7, 2016

OK, first of all there's a change in how the NuGet manager works with .NET Core projects. Instead of a "packages" directory that is a part of your solution folder-structure things get stored under the user-home directory. As if that's not enough, the invocation of the MrAdvice Weaver needs to slightly change as well so your old MrAdvice.Targets file requires some modifications.

I decided to create my own Targets file which I import at the end of my .xproj:

<Import Project="MrAdvice.targets" Condition="Exists('MrAdvice.targets')" />

This is what my Targets file contains:

<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

  <PropertyGroup>
    <!-- The following properties should be read from Project files (*.xprj; project.json) -->
    <MrAdviceNetApp Condition="$(MrAdviceNetApp) == ''">netcoreapp1.0</MrAdviceNetApp>
    <MrAdviceTargetType Condition="$(OutputType) == 'Library'">.dll</MrAdviceTargetType>
    <MrAdviceTargetType Condition="$(OutputType) != 'Library'">.exe</MrAdviceTargetType>
    <MrAdviceTarget Condition="$(MrAdviceTarget) == ''">$(OutputPath)$(Configuration)\$(MrAdviceNetApp)\$(AssemblyName)$(MrAdviceTargetType)</MrAdviceTarget>

    <!-- The following properties should be read from the MrAdvice nuget package -->
    <MrAdviceVersion Condition="$(MrAdviceVersion) == ''">2.4.3</MrAdviceVersion>
    <MrAdviceNetStd Condition="$(MrAdviceNetStd) == ''">netstandard16</MrAdviceNetStd>
    <MrAdviceNuGetPath Condition="$(MrAdviceNuGetPath) == ''">$(USERPROFILE)\.nuget\packages\MrAdvice\</MrAdviceNuGetPath>
    <MrAdvicePath Condition="$(MrAdvicePath) == ''">$(MrAdviceNuGetPath)$(MrAdviceVersion)\</MrAdvicePath>
    <MrAdviceWeaver Condition="$(MrAdviceWeaver) == ''">$(MrAdvicePath)build\MrAdvice.Weaver.exe</MrAdviceWeaver>
    <MrAdviceDLL Condition="$(MrAdviceDLL) == ''">$(MrAdvicePath)lib\$(MrAdviceNetStd)\MrAdvice.dll</MrAdviceDLL>
  </PropertyGroup>

  <UsingTask TaskName="MrAdviceTask" AssemblyFile="$(MrAdviceWeaver)" />
  
  <Target Name="MrAdviceTarget" AfterTargets="CoreCompile" Outputs="@(IntermediateAssembly[0]).♥MrAdvice">
    <!-- The properties sent to MrAdvice.Weaver task may require tweaking? -->
    <MrAdviceTask
      AssemblyPath="$(MrAdviceTarget)"
      ReferencePath="$(MrAdviceDLL)"
      ReferenceCopyLocalPaths="@(ReferenceCopyLocalPaths)"
      AssemblyOriginatorKeyFile="$(AssemblyOriginatorKeyFile)"
      SignAssembly="$(SignAssembly)" />
  </Target>

</Project>

As you can see, I am using NetStandard16, and I'm happy to say that the same Targets file works for DNC App projects as well as DNC WebAPI projects! 😀

@DBCI
Copy link

DBCI commented Dec 7, 2016

I've included two test DNC projects that use MrAdvice... enjoy 😉
AoPDnCSamples.zip

@DBCI
Copy link

DBCI commented Dec 15, 2016

Hi Pascal,

Just wanted to check if there are any new developments regarding MrAdvice and .NET Core projects (NetAppCore1.0/NetStandard16)?
Specifically if there's going to be a MrAdvice version (planned or otherwise) that will correctly integrate into the .NET Core project straight from the NuGet package?

Thanks,
DB

@picrap
Copy link
Member Author

picrap commented Dec 15, 2016

I'd love to! However, regarding all compatibility issues (and perhaps a part of laziness), I'll probably target NET Standard 2.0 only.

@picrap
Copy link
Member Author

picrap commented Dec 15, 2016

But if you're willing to help, you're welcome!

@DBCI
Copy link

DBCI commented Dec 15, 2016

Then until NetStandard v2.0 comes, I'll continue to use my MrAdvice.targets file in my .NET Core projects.

Thanks for everything! 😄

@DBCI
Copy link

DBCI commented Dec 20, 2016

OK, so I'm trying to figure out how to simplify and automate my MrAdvice.targets file and add it to a NuGet package/template... but I noticed something funny. I keep getting a warning when my projects include references to other DLL's.
For example:
MrAdvice.targets(24,5): warning : Can't load 'Dapper, Version=1.50.2.0, Culture=neutral, PublicKeyToken=null'

I assume what I'm doing wrong is passing only the reference to my own DLL instead of all the references participating in the project, but I'm not sure how to do that. Do ReferencePath or ReferenceCopyLocalPaths have anything to do with this?

@picrap
Copy link
Member Author

picrap commented Dec 20, 2016

Don't mind about this, I have the same issue, and I need to solve it... I think I may have left some debug information somewhere...

@DBCI
Copy link

DBCI commented Dec 20, 2016

I'm not worried about this regarding DLL's that are not supposed to contain my Advice attributes, but what about projects that have several DLL's in them that do contain multiple Advice attributes between them?

@picrap
Copy link
Member Author

picrap commented Dec 20, 2016

Does this happen?

@picrap
Copy link
Member Author

picrap commented Mar 21, 2019

Not sure of this, because dotnet build also exists on other platforms where msbuild does not.

@AlexanderButs
Copy link

AlexanderButs commented Mar 21, 2019

At first peek it does. You can check it here https://github.com/dotnet/cli/blob/master/src/dotnet/commands/dotnet-build/BuildCommand.cs

But it does this not via msbuild executable itself but via dotnet and MSBuild.dll

@soroshsabz
Copy link
Contributor

I have the same issue on Appveyor with MrAdvice 2.8.8

What is the last update to this issue? and is there any workaround about it?

@picrap
Copy link
Member Author

picrap commented Jan 9, 2020

We need help for this. First of all, the weaver should be able to run from command-line. Could you try? The documentation is here: https://github.com/ArxOne/MrAdvice/wiki/CommandLineWeaver

@soroshsabz
Copy link
Contributor

No it couldn't run from command line, because

Could not load file or assembly 'Microsoft.Build.Utilities.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'

@picrap
Copy link
Member Author

picrap commented Jan 9, 2020

Could you try with https://www.nuget.org/packages/MrAdvice/2.8.9-embed1 ? I've embedded some of required Microsoft assemblies.

@soroshsabz
Copy link
Contributor

Another issue is I think dotnet/msbuild#4786 we must change MrAdvice.NetStandard project type.

@picrap
Copy link
Member Author

picrap commented Jan 9, 2020

What problem do you have? Could you provide a log or anything that shows the error?

@soroshsabz
Copy link
Contributor

If you open this project in visual studio 2019 you can see the error like below

Portable\v5.0\Microsoft.Portable.CSharp.targets" was not found. Confirm that the path in the declaration is correct, and that the file exists on disk.

@soroshsabz
Copy link
Contributor

I add PR #156 to resolve some issue in building this project under Visual Studio 2019 and netstandard 2

@soroshsabz
Copy link
Contributor

Could you try with https://www.nuget.org/packages/MrAdvice/2.8.9-embed1 ? I've embedded some of required Microsoft assemblies.

@picrap I upgrade my package to 2.8.9-embed1 but does not change anything

C:\Users\sooro\.nuget\packages\mradvice\2.8.9-embed1\build\MrAdvice.targets(10,5): error MSB4062: The "MrAdviceTask" task could not be loaded from the assembly C:\Users\sooro\.nuget\packages\mradvice\2.8.9-embed1\build\..\build\MrAdvice.Weaver.exe. Could not load file or assembly 'Microsoft.Build.Utilities.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'. The system cannot find the file specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [C:\Users\sooro\Documents\Visual Studio 2019\Projects\soroshsabz\Commons\Source\BSN.Commons\BSN.Commons.csproj]

@soroshsabz
Copy link
Contributor

soroshsabz commented Jan 10, 2020

@picrap I think the problem is dotnet/msbuild#2111 (comment) and we must create multiple Weaver.exe for each platform

@picrap
Copy link
Member Author

picrap commented Jan 12, 2020

For information, I'm working on it. I'm currently moving the source library, StitcherBoy to pure .NET Standard, without any dependency, and after this, I'll embed the missing assemblies (Microsoft.Build.*) in MrAdvice package (still built for .NET 4.x (==.NET Standard 2.x), but without dependency)

@AlexanderButs
Copy link

Hey, Pascal, look at my forks. They are already ported to .net standard. I'm not sure embedding Microsoft.Build.* assemblies is a good idea, they may have another dependencies which require full net framework.

@picrap
Copy link
Member Author

picrap commented Jan 12, 2020

OK, "embedding" was the wrong term. I meant "include the dependencies as packages".

@picrap
Copy link
Member Author

picrap commented Jan 12, 2020

Also, I'm removing all references to Microsoft.Build.* in StitcherBoy, so it won't have any constraint.

@AlexanderButs
Copy link

But this won't work in .net core universe as I remembered.

@AlexanderButs
Copy link

Oh, I'm sorry. Yes, you need to consume nugets for net core and just reference assemblies for full framework. See my fork, it works.

@picrap
Copy link
Member Author

picrap commented Jan 12, 2020

Strange, because Microsoft packages depend on .NET Standard 2.0 and .NET framework 4.7.2. .NET Standard is supported by .NET Core, so it should work, shouldn't it?

@AlexanderButs
Copy link

I can make a pull request if you'd like. Just check my fork and if everything is ok i'll do this tomorrow.

@picrap
Copy link
Member Author

picrap commented Jan 12, 2020

I'd like to see if all dependencies can be removed from StitcherBoy, this is what I'm working on right now.

@AlexanderButs
Copy link

Great! Tell me if you need any help :)

@picrap
Copy link
Member Author

picrap commented Apr 26, 2021

@AlexanderButs it works, you can use version 2.9 with dotnet build

@AlexanderButs
Copy link

Hey, Pascal.

Unfortunately I can't verify your changes as I switched project and don't work with MrAdvice any more. But nice to hear you've done this. Good job!

Alex.

@soroshsabz
Copy link
Contributor

@picrap Thanks for attention, but my question is which PR arrive this feature?

@picrap
Copy link
Member Author

picrap commented May 12, 2021

It is already working in release 2.9

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

No branches or pull requests

5 participants