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? #2

Open
DBCI opened this issue Dec 6, 2016 · 37 comments
Open

.NET Core Support? #2

DBCI opened this issue Dec 6, 2016 · 37 comments

Comments

@DBCI
Copy link

DBCI commented Dec 6, 2016

Hello Pascal :)

I hate to be a pest, but do you think there's any chance for .NET Core support for this?

Thank you for the time and effort!
DB

@picrap
Copy link
Owner

picrap commented Dec 6, 2016

Well... That was not planned at all 😄
StitcherBoy is used to created build tools, and don't understand why build tools would need to work with .NET Core (but there are a lot of things I don't understand...).
Regarding .NET Core, I am waiting for .NET Standard 2.0 which will be supported by .NET Core. So for me, a more efficient move (if I make it) is to move to .NET Standard.

@DBCI
Copy link
Author

DBCI commented Dec 6, 2016

Currently, if I try to add (as a NuGet package) either MrAdvice or StitcherBoy to a .NET Core application, the action fails.

Perhaps all that's needed is re-packaging of the tools in a way that would support a DNC project but I don't know if that's even possible.

The one thing I cannot do is wait... but that's my problem. ;)

@picrap
Copy link
Owner

picrap commented Dec 6, 2016

MrAdvice can run as a command-line application (this is totally undocumented, but was very helpful to debug it). However, you'll need to do some plumbing by yourself, such as setting the post-build step and parameters.
The parameters are the same as the MSTask, so this will look like MrAdvice.Weaver AssemblyPath=... References=...

@DBCI
Copy link
Author

DBCI commented Dec 6, 2016

OK, that sounds like a good start! 😄 but... correct me if I'm wrong here, wouldn't I still need to reference the MrAdvice DLL to create my advices, pointcuts etc from within my DNC source code? 😨

@picrap
Copy link
Owner

picrap commented Dec 6, 2016

Yes, you need the reference to MrAdvice.dll, but this can also be made manually (by downloading the package from nuget, extracting it, and referencing the right dll).
That's the poor man's solution, but probably better that nothing...

@DBCI
Copy link
Author

DBCI commented Dec 6, 2016

I'm going to try this and hopefully get out of your hair... 😃

@DBCI
Copy link
Author

DBCI commented Dec 6, 2016

OK, so here's an update:

I managed to activate the MrAdvice Weaver Task properly on my DNC project! 😄 That's the good news because StitcherBoy doesn't require changes. However... I ran into exactly the same problem as I did with MrAdvice, i.e. MrAdvice.Weaver couldn't properly load the MrAdvice DLL!

Is it possible that it's not working because @(ReferencePath) and @(ReferenceCopyLocalPaths) are coming up empty?? 😨

@DBCI
Copy link
Author

DBCI commented Dec 6, 2016

Another quick update... I passed MrAdvice.dll in the ReferencePath parameter and it looks like the Weaver has done it's job... that's the good news. The bad news is that the project doesn't run, but I'll report this as a MrAdvice issue.

I think you can close this issue if you'd like...

@AlexanderButs
Copy link

Hey, Pascal.

Is still necessary to do to migrate MrAdvice to .NET Core? What do you think?

I've done some work by migrating StitcherBoy to netstandard2.0 here https://github.com/AlexanderButs/StitcherBoy and loaded this package as a nuget into MrAdvice as a netstandard2.0 dependancy. Could you have a look on my changes and maybe i'll send a PR? (But you will need to do some work related to nuget release).

Also we can discuss further work to do to migrate MrAdvice to .NET Core - as I understand I need to migrate Blobber as well to be able to pack MrAdvice into one package. Am I right?

Alex.

@picrap
Copy link
Owner

picrap commented Aug 2, 2019

Hi Alexander, I understand the lot of forks you made recently 😉

How would consider the final Mr Advice for .NET Core? Can it run as a build task? How otherwise? Once we agree on where we need to go, we'll try to see how to go there.

@AlexanderButs
Copy link

Yeah, we are in .NET Core transition state and I have some time to help you with this library so why not :)

Idea is the next - MrAdvice will work the same as it does right now. Everything I need to do is to migrate actual MrAdvice.dll to support net461 and netstandard2.0, make MrAdvice.Weaver.exe compatible with net461 and netstandard2.0 (put these to two separate folders) and use proper weaver inside msbuild task depending on framework target you are building to. For all that stuff we need to migrate StitcherBoy and Blobber as well as I understand. StitcherBoy is almost done by me - need to test a little bit on your side and release to nuget. Your thoughts? :)

@picrap
Copy link
Owner

picrap commented Aug 2, 2019

How about the references to Microsoft.Build assemblies? Are they still available?

@AlexanderButs
Copy link

Yes, they are. You can look inside my fork how they are used for different target frameworks. I use conditional reference there.

@picrap
Copy link
Owner

picrap commented Aug 2, 2019

OK, so I guess moving to .NET Standard is the smart thing to do :) I'm currently reviewing your changes

@AlexanderButs
Copy link

I don't use the latest Microsoft.Build* packages as they don't target netstandard2.0 unfortunately. I guess they are (Microsoft) in transition state as well and in the future they will target netstandard2.1 or something like that.

@picrap
Copy link
Owner

picrap commented Aug 2, 2019

I reviewed you code, there are some hard-coded version numbers, except that this is probably OK.
EDIT: in StitcherBoy

@AlexanderButs
Copy link

I will have a look. Thanks :)

@AlexanderButs
Copy link

What version number? Referenced packages or StitcherBoy library itself?

@picrap
Copy link
Owner

picrap commented Aug 2, 2019

"1.3": StitcherBoy version in .nuspec file

@AlexanderButs
Copy link

Yeah, my bad. Thanks for notice :) Will fix that and will make a PR.

@AlexanderButs
Copy link

I brought back all assembly information to AssemblyInfo.cs and ProductInfo.cs except version number. Also a small copy paste of author and description inside nuspec file because of NuGet/Home#4234

@AlexanderButs
Copy link

Also it's needed to change deployment script because right now nuget package should be created by this command: dotnet pack StitcherBoy.csproj /p:NuspecFile=StitcherBoy.nuspec -c Release

@AlexanderButs
Copy link

Hello, Pascal.

My update: I made MrAdvice works in dotnet core environment :) To do that I ported StitcherBoy and Blobber to netstandard2.0 and made necessary changes. Also I ported MrAdvice itself to netstandard2.0+net461. I tested it out on test projects and it works fine for both netstandard2.0, netcoreapp2.0 and net461.

But I have two major problems:

  • It doesn't work for multi-target projects when you want to build some project for netstandard2.0 and net461 - weaver doesn't unload some dependency and fails.
  • I couldn't make tests work. I ported ExternalAdvices and MethodLevelTest projects to dotnet core and spent two whole days to weave unit test project but unsuccessfully :( I couldn't get any debug logs as well :(

Also I didn't work with build process related to appveyor work. I guess it should be done in the very end.

Do you have some time to help me with that? I pushed all my changes to my forks and they should work. If you have some time I can tell you how to build all nuget packages and make MrAdvice package.

@picrap
Copy link
Owner

picrap commented Sep 5, 2019

What happens on failing tests? Maybe the weaving does not work correctly? It could be worth the try upgrading dnlib in StitcherBoy.

@AlexanderButs
Copy link

It fails during building stage. Weaving target just fails and that's all. Without any logs. I tried to pass "-v d" to msbuild to get full logs but no result :(

@picrap
Copy link
Owner

picrap commented Sep 5, 2019

This may be related to dnlib, so try upgrading it in StitcherBoy and then rebuild MrAdvice.

@AlexanderButs
Copy link

ok, i'll try this. thanks for the hint.

@AlexanderButs
Copy link

Unfortunately, it doesn't help. I've got next with detailed logging during build unit test project:
Target "MrAdviceWeaver" in file "E:\Work\MrAdvice\MrAdvice.Weaver\MrAdvice.targets" from project "E:\Work\MrAdvice\Test\MethodLevelTest\MethodLevelTest.csproj" (target "Compile" depends on it):
Building target "MrAdviceWeaver" completely.
No input files were specified.
Using "MrAdviceTask" task from assembly "E:\Work\MrAdvice\MrAdvice.Weaver..\build\MrAdvice.Weaver.dll".
Task "MrAdviceTask"
Done executing task "MrAdviceTask" -- FAILED.
1:7>Done building target "MrAdviceWeaver" in project "MethodLevelTest.csproj" -- FAILED.

@picrap
Copy link
Owner

picrap commented Sep 5, 2019

There is a catch that mutes exceptions (as far as I remember) in StitcherBoy. Something probably happens there, you need to log the exception so you can diagnose it.

@AlexanderButs
Copy link

I added some logging into the very beginning of weaving process but didn't get any logs.

@picrap
Copy link
Owner

picrap commented Sep 6, 2019

So we can probably guess that the assembly load fails, on the assembly itself or dependencies. How do you test the weaver? Do you specify dependencies explicitly?

@AlexanderButs
Copy link

Probably, and I don't know how to investigate that :(
Yes, as you do it in .net framework version i do it the same - direct ref to the mradvice abstractions and direct weaver target use. You can see it here https://github.com/AlexanderButs/MrAdvice/blob/master/Test/MethodLevelTest/MethodLevelTest.csproj

@AlexanderButs
Copy link

AlexanderButs commented Sep 6, 2019

The weaving process itself I tested on a test project - create new nuget packages for all 3 projects (blobber, stitcherboy and mrweaver) and use it in the test project. Everything works fine there.

@picrap
Copy link
Owner

picrap commented Sep 6, 2019

How can I help? Is the project where the error can be seen avaible?

@AlexanderButs
Copy link

AlexanderButs commented Sep 9, 2019

Maybe you know how to investigate those issues? And sometimes it's useful to show work to someone else :)
Yes, I have forks for all three projects (Blobber, StitcherBoy and MrAdvice). You need to increase nuget versions in nuget spec files for Blobber and StitcherBoy and create nugets for them with next command "dotnet pack Blobber.csproj /p:NuspecFile=Blobber.nuspec -c Release". Don't forget to use new Blobber nuget version in StitcherBoy. Then you need to use produced nugets in MrAdvice project and try to build MethodLevelTest project with "dotnet build -c Release". If you have any questions feel free to ask and I'll help you with all that stuff.

Thanks for you help in advance :)

@soroshsabz
Copy link

I think it is good to change this project to netstandard 2 or 2.1, for this work we must remove microsoft.build.evaluation dependencies or we must find some alternative for it.

@soroshsabz
Copy link

ref ArxOne/MrAdvice#71

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

4 participants