proj-info icon indicating copy to clipboard operation
proj-info copied to clipboard

Resolve Framework and COM references

Open dsyme opened this issue 3 years ago • 11 comments
trafficstars

Loading a project should fully resolve framework and COM references, see #171

Tests needed

dsyme avatar Sep 27 '22 17:09 dsyme

@baronfel I tried to look into the test failures but I'm finding the engineering in this repo a bit difficult.

  • How do we actually see the output of tests? I'm not familiar with Expecto.
  • I couldn't successfully use ionide-proj-info.sln in Visual Studio - lots of things about unresolved assemblies and this

image

dsyme avatar Sep 27 '22 17:09 dsyme

expecto runs via dotnet run on the test project, and can be filtered with --filter "TEST NAME PATTERN". There's a dotnet tool that you can run against a project with dotnet run as well to do things more interactively.

Due to the MSBuild dependency, many more traditional dotnet test wrappers like nunit/xunit don't work well (the xunit runner enforces a local dependency on Microsoft.Build.Framework, for example).

baronfel avatar Sep 27 '22 17:09 baronfel

@baronfel I see thanks.

Given how core this component is I do wonder if we should simplify a few things - e.g. remove Paket, remove Ionide.KeepAChangeLog whatever that is.

dsyme avatar Sep 27 '22 17:09 dsyme

Paket is a potential for removal, but the Ionide.KeepAChangelog dependency is a valuable way to ensure that version numbers and release notes are a) kept, and b) included in all assembly and nuget package outputs automatically.

baronfel avatar Sep 27 '22 17:09 baronfel

but the Ionide.KeepAChangelog dependency is a valuable way to ensure that version numbers and release notes are a) kept, and b) included in all assembly and nuget package outputs automatically.

OK, though it fails in VisualStudio, not sure why. Removing it through makes the solution loadable in latest VS 2022 Preview

dsyme avatar Sep 27 '22 17:09 dsyme

Correct, this is a known issue logged on that repo. If we moved off of Paket it would be very easy to conditionally include it based on if the user is accessing from VS (at least until the bug is fixed).

baronfel avatar Sep 27 '22 17:09 baronfel

I think what's going on with the tests is that some projects (.NET SDK projects?) may not have some of the targets based on TFM - some of the new targets may need to pivot on TFM.

baronfel avatar Sep 27 '22 17:09 baronfel

I tried these:

dotnet run --framework net6.0 --project test\Ionide.ProjInfo.Tests\Ionide.ProjInfo.Tests.fsproj
dotnet run --framework net7.0 --project test\Ionide.ProjInfo.Tests\Ionide.ProjInfo.Tests.fsproj

The first failed various tests with some errors due to .NET 7, the second with this:

C:\Program Files\dotnet\sdk\7.0.100-rc.1.22431.12\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolution.
targets(267,5): error NETSDK1005: Assets file 'C:\GitHub\dsyme\proj-info\test\Ionide.ProjInfo.Tests\obj\project.assets.
json' doesn't have a target for 'net7.0'. Ensure that restore has run and that you have included 'net7.0' in the Target
Frameworks for your project. [C:\GitHub\dsyme\proj-info\test\Ionide.ProjInfo.Tests\Ionide.ProjInfo.Tests.fsproj::Target
Framework=net7.0]

dsyme avatar Sep 27 '22 17:09 dsyme

@dsyme I've got this change on another PC, but we need to add a 7.0 TFM to the tools project and rerun dotnet paket install, then you should be all clear.

baronfel avatar Sep 27 '22 17:09 baronfel

Well, belay that a bit - you need to set BuildNet7 to true in your local environment. There are reasons for this related to runtime selection by the SDK, but this is how we do the 7.0 testing in CI.

baronfel avatar Sep 27 '22 17:09 baronfel

@baronfel OK thanks. I'll have to leave this for a while but does the change here look right? I'm really surprised we haven't needed this before. How else are our tools all getting framework references?

dsyme avatar Sep 27 '22 17:09 dsyme

Closing as https://github.com/ionide/proj-info/pull/177 should fix this

TheAngryByrd avatar Nov 07 '22 03:11 TheAngryByrd