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

WIP: Optionally remove dependency on MSBuild reference resolution #649

Merged
merged 23 commits into from Oct 13, 2016

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Oct 12, 2016

I'm looking at removing the forced dependency on MSBuild for reference resolution (currenty MSBuild v12) and giving resasonable behaviour when MSBuild is not installed.

FCS only uses MSBuild reference resolution for referenced in scripts and direct calls to the F# compiler (it is not used when invoked via Visual Studio or Xamarin - except for scripts). Even then it is only for references like #r "System.Xml or #r "Sytem.Xml, Version=4.3.2.1,..." etc, which are not full assembly names but rather FullName specifications. (These#rspecs can also be given on thefsc.exe`` command line, though no one ever uses that).

The idea in this PR is that if the flag msbuildEnabled=false is given to FSharpChecker or FsiEvluationSession then a more simplistic reference resolution mechanism (SimplisticResolver) is used which simply searches paths and registry keys (and perhaps eventually the GAC) trying to emulate the behaviour of MS Build reference resolution.

Implementation-wise, we factor the MSBuild reference resolution into a separate DLL (perhaps we will need one for each version of MSBuild we may want to interoperate with). This DLL is invoked by reflection by default. For the ones without versions like #r "System.Xml", the resolution strategy in the PR should be adequate I think.

The PR still uses MSBuild v12 by default if installed. But if Microsoft.Build.Framework 12.0.0.0 can't be loaded using Assembly.Load, then the component is not accessed and it defaults back to the SimplisticResolver resolver.

There are still sure to be glitches in SimplisticResolver - though if it passes FCS tests that's a pretty good indication that it's usable as a backoff strategy.

@dsyme
Copy link
Contributor Author

dsyme commented Oct 12, 2016

Note this doesn't remove the use of MSBuild for project file cracking. But for deploying F# compilation that only uses scripts it should be adequate to allow the use of FCS with no MSBuild dependency

An FSPROJ project cracker that has no MSBuild dependency would be another step towards our liberation. But that is very difficult (needs a whole MSBuild interpreter) and can depend on significant MSBuild internals.

@nosami
Copy link
Member

nosami commented Oct 12, 2016

This looks great!
At Xamarin, we don't use the project cracker any more (Xamarin Studio has it's own msbuild parser). I recently integrated FCS into Xamarin Workbooks and needed to pull in Msbuild.Tasks/Engine/Framework etc references so will be happy to remove those.

I'll give this a test tomorrow. I just need to make sure that #r System.Xml etc. work using the simple resolution?

@rneatherway
Copy link
Member

Do you think it would be possible to use the Msbuild parser from XS here in
FCS?

On 13 Oct 2016 12:24 a.m., "Jason Imison" notifications@github.com wrote:

This looks great!
At Xamarin, we don't use the project cracker any more (Xamarin Studio has
it's own msbuild parser). I recently integrated FCS into Xamarin Workbooks
and needed to pull in Msbuild.Tasks/Engine/Framework etc references so will
be happy to remove those.

I'll give this a test tomorrow. I just need to make sure that #r
System.Xml etc. work using the simple resolution?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#649 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAO5G1JwICuXhrFXPtwOAjfxWUWK3oTyks5qzWwmgaJpZM4KVT5a
.

@nosami
Copy link
Member

nosami commented Oct 12, 2016

@rneatherway Not easily - the XS parser uses msbuild anyway, so you wouldn't gain anything.

@thinkbeforecoding
Copy link
Contributor

What kind of scenario does msbuild resolver solve that the simplistic resolver won't ?

@dsyme
Copy link
Contributor Author

dsyme commented Oct 13, 2016

What kind of scenario does msbuild resolver solve that the simplistic resolver won't ?

@thinkbeforecoding When a versioned reference is being used like #r "EventViewer, Version=6.3.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35" I believe the MSBuild resolver checks precise versions when searching files. However using long name versioned references in scripts is rare.

More problematic may be discrepancies, e.g. #r "EventViewer" may resolve to one version with the MSBuild resolver, and another version withSimulatedMSBuildResolver(which is what I'm now callingSimplisticResolver``, since it's attempting to do what we did before just with no explicit dependency on MSBuild)

I assume the MSBuild reference resolver also checks process architecture, so if you're running FSI 32-bit it will find 32-bit DLLs should the DLLs be marked with process-specific architecture. I don't know if this is a problem in practice for F# scripts but I assume it could be.

In other words I believe we can emulate the basic search that we ask the MSBuild reference resolver to perform, but there may be glitches in rarely-used corner cases.

@dsyme
Copy link
Contributor Author

dsyme commented Oct 13, 2016

@thinkbeforecoding There is also a risk that simulated MSBuild reference resolution will be slower for the cases where it is used. I don't think this will be an issue though, since in most scenarios (Xamarin, Visual Studio) the client pre-resolves, and in other scenarios using FSharpChecker the resolution is not repeated unless the reference set changes

@dsyme
Copy link
Contributor Author

dsyme commented Oct 13, 2016

open Microsoft.FSharp.Compiler.ReferenceResolver
open Microsoft.FSharp.Compiler.AbstractIL.Internal.Library

let internal SimulatedMSBuildResolver =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be exposed? I'm trying to remove the ProjectCracker dependency using Forge.ProjectSystem but I need a way to resolve dependencies. This would be very useful to resolve the core dependencies of .fsproj file.

@dsyme
Copy link
Contributor Author

dsyme commented Oct 24, 2016

@alfonsogarciacaro I suppose so, please submit a PR?

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.

None yet

5 participants