Skip to content

Conversation

@brettfo
Copy link
Member

@brettfo brettfo commented May 17, 2018

Followup to #4922.

By previously setting FSharpTargetsPath, etc. directly we didn't allow for that to be manually set in a project, props, or target file. By using this method the IDE can communicate where the compiler is located, but the user can still override this should they choose.

Edit:
With this change compiler redirection will require a shipped version of the targets shim changes which will happen in VS 15.8.

Copy link
Member

Choose a reason for hiding this comment

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

But nothing will install to this path anymore, since we removed the MSI?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll have to rebase that PR to fix this bottom line, but this will still work. The MSI removal doesn't remove these files, they're just placed under Common7\IDE\....

@Pilchie
Copy link
Member

Pilchie commented May 17, 2018

I guess my overall point is that I'd prefer us to match C# as closely as we can for this stuff (keeping in mind that FSharp.Core throws a small wrench here). I acknowledge that we still need a good way of having the F5'd vsix use the new compiler, but I'd prefer the shipped product mirror C#.

@cartermp cartermp added this to the 15.8 milestone May 20, 2018
@0x53A
Copy link
Contributor

0x53A commented May 30, 2018

FSharp.Compiler.Tools overrides the compiler using this property and mirrors Microsoft.Net.Compilers.

Please continue to support both.

@brettfo
Copy link
Member Author

brettfo commented May 30, 2018

@0x53A Thanks for the pointer. Looking at how FSharp.Compiler.Tools.props works it should be good, but I'll modify my local install of VS to match what this PR does and verify just to be sure.

@brettfo
Copy link
Member Author

brettfo commented May 30, 2018

@0x53A It works as expected. I did the following:

  1. Built these changes locally and deployed the VSIX (to ensure I have the code that sets FSharpCompilerPath on project load).
  2. Modified my local targets files to have the redirect that exists in this PR.
  3. Built from VS -> fsc was invoked from the deployed VSIX location.
  4. I added a reference to FSharp.Compiler.Tools version 10.0.2.
  5. Re-built from VS -> fsc was invoked from ..\packages\FSharp.Compiler.Tools\...
  6. Re-built from the command line -> same as in step 5.

@brettfo brettfo merged commit 0b940a4 into dotnet:master May 30, 2018
@brettfo brettfo deleted the compiler-redirect-redux branch May 30, 2018 22:55
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.

5 participants