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

FSharp.Compiler.Private --> .NET Standard 2.0 #7717

Closed
wants to merge 2 commits into from
Closed

FSharp.Compiler.Private --> .NET Standard 2.0 #7717

wants to merge 2 commits into from

Conversation

cartermp
Copy link
Contributor

@cartermp cartermp commented Oct 9, 2019

Trying again since #7220 just seems borked

@cartermp
Copy link
Contributor Author

cartermp commented Oct 10, 2019

This is blocked because this fails now that the project is NS 2.0 only:

type ResolveAssemblyReference () =
let assembly = Assembly.Load(new AssemblyName("Microsoft.Build.Tasks.v4.0, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a"))
let resolveAssemblyReferenceType = assembly.GetType("Microsoft.Build.Tasks.ResolveAssemblyReference")
let instance = Activator.CreateInstance(resolveAssemblyReferenceType)

Infinite sadness. @KevinRansom @dsyme any thoughts on how to get this stuff out of the compiler forever?

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2019

This is blocked because this fails now that the project is NS 2.0 only:

What actually fails? Does that code compile o is something missing in NS 2.0?

@cartermp
Copy link
Contributor Author

It all builds (and runs) just fine. 400+ tests fail though, since the apparently have this implicit depedency on reflection loading a very old MSBuild .dll.

@dsyme
Copy link
Contributor

dsyme commented Oct 11, 2019

It all builds (and runs) just fine. 400+ tests fail though, since the apparently have this implicit depedency on reflection loading a very old MSBuild .dll.

Do you know what the exception is? I mean, the reflection loading cod above should still work even if this component is .NET Standard?

@cartermp
Copy link
Contributor Author

In this case you can see the first failure in CI logs here: https://dev.azure.com/dnceng/public/_build/results?buildId=383148&view=ms.vss-test-web.build-test-results-tab&runId=11832352&resultId=100040&paneView=debug

The reflection code likely doesn't work, hence we get a null instance that we null-ref on in tests. IIRC none of this code is run on .NET Core, so to me it just feels like some legacy to push out

@dsyme
Copy link
Contributor

dsyme commented Oct 11, 2019

The reflection code likely doesn't work, hence we get a null instance that we null-ref on in tests. IIRC none of this code is run on .NET Core, so to me it just feels like some legacy to push out

Right, that's what's odd - this is still running on .NET Framework.

I assume it's actually a difference in the #if conditions causing variation in code being run

@KevinRansom
Copy link
Member

The issue is it shouldn't really be going through any reshaped msbuild code. On the coreclr, it isn't really used except to keep the compiler happy, on the desktop it uses the real msbuild.

Now that we have msbuild dll's for the coreclr we need to work through moving over to it, but there are some differences I think between the netcore versions and the old desktop versions. We need to work through those. It will also require developers to deploy the msbuild dll's.

Right now, the notebook is a bit of a priority, but cleaning up the msbuild part of our code is a long cherished dream.

@cartermp
Copy link
Contributor Author

Closing since I deleted my fork

@cartermp cartermp closed this Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants