VirtualPhotonics / VTS

Virtual Tissue Simulator
https://virtualphotonics.org
Other
34 stars 9 forks source link

VTS OOB developer story - desktop dependencies out of date. Vts.Desktop necessary anymore? #10

Closed dcuccia closed 6 years ago

dcuccia commented 6 years ago

I added the VirtualPhotonics.VTS NuGet package, and wrote a .Net Core console test app to exercise the VTS.MonteCarloSimulation class:

using System;

namespace TestVts
{
    class Program
    {
        static void Main(string[] args)
        {
            RunASimpleMonteCarlo();
        }

        private static void RunASimpleMonteCarlo()
        {
            var input = new Vts.MonteCarlo.SimulationInput();

            var mc = new Vts.MonteCarlo.MonteCarloSimulation(input: input);

            var output = mc.Run();

            Console.Write($"Simulation results:" +
                $"\r\n\t{input.N} photons generated" +
                $"\r\n\t{output.R_r_TallyCount} photons detected");

            Console.ReadKey();
        }
    }
}

This works great. I then wanted to improve the VTS library, so I checked out the source code in a new repository, made some changes, built the project, and then referenced all .dll libraries in the following folder:

...\build\core\Desktop\Release

Repeating the execution, I get a runtime exception in LoggerFactory.cs:

vts system reactive error

Looks like the source code is using individual System.Reactive pieces (Core, Interfaces, Linq, PlatformServices), but the runtime expects the "full" System.Reactive. Removing the individual pieces, and instead adding System.Reactive 4.1.0 via NuGet, makes everything happy again.

I did a little digging, and discovered that we only copy 'Vts.Desktop' builds to the 'build' folder, but the netstandard 2.0 'Vts' libarary isn't copied. So, I was accidentally referencing "desktop" assets from a core project (which again, worked otherwise, just needed references updated).

I'd suggest two things:

1) Decide if the Vts.Desktop project is needed anymore. All desktop projects are on .Net 4.6.1 and thus netstandard 2.0 compliant. Is there a reason to keep? Desktop projects should be able to just reference the Vts library directly, and therefore work with Desktop, Core, Mono, Xamarin runtimes.

2) Figure out the story for packaging netstandard projects for re-use per my scenario above, where all I want to do is reference the compiled Vts.dll and start programming against it. I see that there's a Vts.deps.json file saved alongside the netstandard Vts.dll file. Presumably, referencing projects can reference this 'deps' file and have the tooling automatically pull in the necessary dependencies. Supporting this scenario will significantly improve other's ability to work with the API and make/suggest improvements, based on their specific use-cases.

hayakawa16 commented 6 years ago

Hi dcuccia,

Thanks for your feedback. Unfortunately, we just entered a transition period and you have caught some of our recent change errors. I'm sorry about that. Currently the Vts.Desktop project is needed for the WPF GUI. We are currently working on Xamarin Forms (XF) version and when this is complete, there will be no need for the desktop project. The XF solution cannot have multiple copies of libraries within the solution and there were conflicts with those libraries brought in by the resident XF code and those brought in by our Vts, so we have been trying to eliminate those packages that are not necessary to the Vts solution to enable the XF solution to compile using the Vts net standard nuget package. Keeping the Vts net standard, Vts.Desktop, the WPF GUI, and their associated unit tests, and the XF project all updated, compiling and passing unit tests has been a bit of a challenge lately, especially since many of the nuget packages we use get updated on a weekly basis and those bring in subtle changes.

So I appreciate your suggestions. If we can simplify our process so that when we push on one side something doesn't break on another, that would be great.

We'll get there. Your input will help us. So thanks again.

dcuccia commented 6 years ago

Interesting - curious why the WPF GUI won't work with the Vts project. My understanding is that a .Net 4.6.1 app should be able to consume a .Net Standard 2.0 library.

On Mon, Sep 3, 2018, 5:49 PM Carole Hayakawa notifications@github.com wrote:

Hi dcuccia,

Thanks for your feedback. Unfortunately, we just entered a transition period and you have caught some of our recent change errors. I'm sorry about that. Currently the Vts.Desktop project is needed for the WPF GUI. We are currently working on Xamarin Forms (XF) version and when this is complete, there will be no need for the desktop project. The XF solution cannot have multiple copies of libraries within the solution and there were conflicts with those libraries brought in by the resident XF code and those brought in by our Vts, so we have been trying to eliminate those packages that are not necessary to the Vts solution to enable the XF solution to compile using the Vts net standard nuget package. Keeping the Vts net standard, Vts.Desktop, the WPF GUI, and their associated unit tests, and the XF project all updated, compiling and passing unit tests has been a bit of a challenge lately, especially since many of the nuget packages we use get updated on a weekly basis and those bring in subtle changes.

So I appreciate your suggestions. If we can simplify our process so that when we push on one side something doesn't break on another, that would be great.

We'll get there. Your input will help us. So thanks again.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/VirtualPhotonics/VTS/issues/10#issuecomment-418211170, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdRgUjTOiznnlSJrCGkqA3EscipIl44ks5uXc4RgaJpZM4WYG0b .

lmalenfant commented 6 years ago

Hi David,

We still plan to keep the .NET Framework version of our software so we can support users who are on older versions of Visual Studio. We would also have to change the way we access the libraries for features like the MATLAB interoperability.

Your suggestion for adding a post build copy of the .NET Standard version of the library to the build folder was a good one. I added the post build command to copy Debug and Release versions of Vts.dll and Vts.deps.json to the build folder however when I try to reference the library from a similar .NET Core command line application, it does not pull the dependencies. Do you have any experience with these types of references?

dcuccia commented 6 years ago

No experience, but there has to be a simple story on how to chain nuget deps, as this should be designed to work across VS code and other tools in a cross-plat way.

Re: old VS I see that for the core library, makes sense for legacy systems (though, not sure who that 'customer' is with free VS Code and VS community). But for all compiled front-end tools, I think it's fair to expect a minimum of 4.6.1 and .NET Standard 2.0

On Wed, Sep 5, 2018, 3:37 PM Lisa Malenfant notifications@github.com wrote:

Hi David,

We still plan to keep the .NET Framework version of our software so we can support users who are on older versions of Visual Studio. We would also have to change the way we access the libraries for features like the MATLAB interoperability.

Your suggestion for adding a post build copy of the .NET Standard version of the library to the build folder was a good one. I added the post build command to copy Debug and Release versions of Vts.dll and Vts.deps.json to the build folder however when I try to reference the library from a similar .NET Core command line application, it does not pull the dependencies. Do you have any experience with these types of references?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/VirtualPhotonics/VTS/issues/10#issuecomment-418903941, or mute the thread https://github.com/notifications/unsubscribe-auth/AAdRgZVtbVCggtEI7QAkmWY4bGUnr6N7ks5uYFJEgaJpZM4WYG0b .

lmalenfant commented 6 years ago

I agree with you that there should be a "simple" way to chain nuget dependencies but after searching many blog posts and articles on this and *.deps.json, I still have not found a way.

Here are a couple of the more useful articles that I found: Scott Hanselman Blog Post Nate McMaster Blog Post

I have tried everything suggested in these posts and more and I still cannot easily reference the .NET Standard library.

dcuccia commented 6 years ago

From the Hanselman post, it seems like we'd just edit one line to the .csproj:

<RestoreProjectStyle>PackageReference</RestoreProjectStyle>

Tried this on my .Net Core command line project and it worked great. Tried on a .NET Desktop command line and it didn't work. This is the error I'm getting:

Referencing a .net standard class library project in .net Framework web app and System.IO.FileNotFoundException throws https://github.com/dotnet/standard/issues/410

...and I think these are the appropriate threads regarding the core issue:

Issues with .NET Standard 2.0 with .NET Framework & NuGet https://github.com/dotnet/standard/issues/481

Dependencies don't flow from new NETStandard project to old Desktop projects through ProjectReferences https://github.com/dotnet/sdk/issues/901

lmalenfant commented 6 years ago

Adding that line to the .csproj file did not work for me, I was still getting missing references. Which folder with the VTS library were you including as a reference? I tried the one in the bin folder as well as the one I just added to the build folder.

dcuccia commented 6 years ago

When it worked on .Net Core, I was referencing src\Bts\bin\Debug\netstandard2.0\Vts.dll. It also has a Vts.deps.json file alongside it with all project dependencies. That said, I just tried on a new machine and couldn't reproduce, so very confused. Need to research more...here are a few breadcrumbs to myself:

https://github.com/NuGet/Home/issues/4488 https://github.com/dotnet/sdk/issues/757

dcuccia commented 6 years ago

...and https://docs.microsoft.com/en-us/nuget/consume-packages/dependency-resolution

dcuccia commented 6 years ago

Figured it out (or at least the work around). TL;DR: in the .Net Standard 2.0 Vts .csproj XML, change:

<TargetFramework>net46</TargetFramework>

to

<TargetFrameworks>netstandard2.0;net46</TargetFrameworks>

More info: Immo Landwerth has a great video from yesterday's dotnetconf event here. At ~ minute 23 into the video, he says there are problems/bugs with .NET 4.6.1 consuming .Net Standard 2.0 libraries. He recommends using .Net 4.7.2 (has to be downloaded separately), but turns out that's not the issue. Instead, just need to edit the XML of the Vts project to do true multi-targeting. The WPF and console applications can then have Vts as a dependency, and there's no need for the separate Vts.Destkop project anymore. At least, that's the theory. We should create a branch and flush this out.

dcuccia commented 6 years ago

Here's the SO post that helped me: https://stackoverflow.com/a/42364427/22528

lmalenfant commented 6 years ago

As per my reasons above we do still have a need for the Vts.Desktop project. The information you provided is very helpful for using the .NET Standard library in .NET Framework projects, thank you for the references.

Until .NET Standard and .NET core are more widely used, we feel more comfortable keeping the .NET Framework version. The code is identical now so for maintenance purposes, it doesn't create any additional work.

dcuccia commented 6 years ago

My proposed change includes .NET desktop build, it just eliminates the need to keep a separate Vts.Desktop project, instead using multi-targeting so that one Vts project results in a .NET 4.6 and .NET Standard library. Could also add 'netcoreapp2.1' and build three libraries with one project.

lmalenfant commented 6 years ago

Added to the .NET Standard 2.0 project as a possible future enhancement.