Migrate new sdk #527
Migrate new sdk #527
Conversation
| @@ -28,11 +28,13 @@ public CompilationResult (DateTime write_time, string result_file) | |||
| } | |||
|
|
|||
| public static class Platform { | |||
|
|
|||
| #if NET_CORE | |||
| public static bool OnMono => Environment.OSVersion.Platform == PlatformID.Unix || Environment.OSVersion.Platform == PlatformID.MacOSX; | |||
There was a problem hiding this comment.
We should use Portable Pdb to make the pdb test work https://github.com/dotnet/core/blob/master/Documentation/diagnostics/portable_pdb.md
|
@jbevain ok I think I m done, If you wan't me to split the upgrade to netstandard 2.0 and sdk I can do it |
|
Why still keeping net35 and net40? According to this comment netstandard2.0 is desired by @jbevain: #507 (comment) EDIT: Oh maybe I misunderstood that comment and it was only about netstandard1.3 vs netstandard2.0. |
|
If it is the case but I don t think It would be easy to remove them |
|
@jbevain Is that a problem that I added a dependency for netstandard? It would be great to add another one to use DependencyContext |
|
@jbevain anything i can help with here? |
|
@bhugot thank you for your work. I'm considering moving to the new project files in master for the 0.11 version. I'm currently exploring the tooling. I don't know yet if I'll merge this branch of take inspiration from it, but thanks again for your contribution! |
|
@jbevain no problem I understand can be big change if PR can help you to migrate, my job is done fill free to contact me if need some help |
| <DefineConstants>$(DefineConstants);READ_ONLY</DefineConstants> | ||
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(TargetFramework)' == 'net35' "> | ||
| <DefineConstants>$(DefineConstants);LEGACY</DefineConstants> |
There was a problem hiding this comment.
Is the LEGACY define used? A quick search in the code doesn't find anything.
There was a problem hiding this comment.
I think I put this at a moment where I was using it but it's no more needed. Could be removed.
| </PropertyGroup> | ||
| <PropertyGroup Condition="'$(TargetFramework)' == 'net35' "> | ||
| <DefineConstants>$(DefineConstants);LEGACY</DefineConstants> | ||
| <FrameworkPathOverride Condition="'$(FrameworkPathOverrideForMono)' != ''">$([System.IO.Path]::Combine($(FrameworkPathOverrideForMono),2.0-api))</FrameworkPathOverride> |
There was a problem hiding this comment.
Why are the FrameworkPathOverride needed, on Mono and not on Mono?
There was a problem hiding this comment.
It's because net35 is not compiled with dotnet build.
| @@ -0,0 +1,5 @@ | |||
| <Project> | |||
There was a problem hiding this comment.
Could you explain the relation between this and Directory.Build.targets? I'm a bit confused at to what those do.
There was a problem hiding this comment.
This is a tips to fix the dotnet test on a solution so that the non tests are not executed and failing microsoft/vstest#1129 (comment)
| </ItemGroup> | ||
| <ItemGroup> | ||
| <Compile Include="Mono.Cecil.Tests\*.cs" /> | ||
| </ItemGroup> | ||
| <ItemGroup> | ||
| <None Include="Resources\**\*" /> | ||
| <None Include="Resources\**\*" > | ||
| <CopyToOutputDirectory>Always</CopyToOutputDirectory> |
There was a problem hiding this comment.
Why do we need to copy the Resources to the output directory?
There was a problem hiding this comment.
I don't remember but I think if I did it it's because it was needed.
|
@bhugot I'm sorry it must seem forever for you, but I've started looking into this, and I added a few comments as there's a few things I don't understand :) |
| var getData = appContextType?.GetTypeInfo ().GetDeclaredMethod ("GetData"); | ||
| paths = (string) getData?.Invoke (null, new [] { "TRUSTED_PLATFORM_ASSEMBLIES" }); | ||
|
|
||
| paths = (string) AppDomain.CurrentDomain.GetData("TRUSTED_PLATFORM_ASSEMBLIES"); |
There was a problem hiding this comment.
@tmat that change is ok with you?
|
@jbevain no problem like I said happy to help |
|
@bhugot I've merged my branch to support .NET Standard 2.0 and the new SDK style projects. Thanks a lot for your work, it's been a great source of inspiration. Because I'll be maintaining this for the foreseeable future, I wanted to do the port manually to familiarize myself with all the differences, but your work has been a great source of inspiration. Thank you! |
|
@jbevain happy you did it and that my work helped you on this. Waiting for the release. |
I duplicated .sln and .csproj with .LegacyProject in name
I upgraded to netstandard2.0
I know there was another PR on this but as you didn't merged it it's my proposal