Projects out of process (new api) #470

rneatherway · 2015-11-27T17:51:01Z

No description provided.

This aims to fix many problems stemming from two main issues: * FCS depends on MSBuild v12, and not all machines have this installed. * It's hard to apply robust assembly redirects, as this is up to the application referencing FCS. These are needed to redirect MSBuild v4 to whatever is being used (for custom MSBuild tasks), and also to allow redirecting to MSBuild v14 if necessary. The API is preserved, with an additional function, `GetProjectOptionsFromProjectFileLogged` to allow easier access to the logs for presentation to users and bug-reporting. The functionality is implemented by FCS calling a new console application, FSharp.Compiler.Service.ProjectCracker.exe, and the response is transmitted via serialization using JSON to types shared by a common source file, so there is no binary dependency between the two. The ProjectCracker static links to FSharp.Core, as it is in the same directory as FCS.dll. Thus bundling a separate copy would force the choice of version on all users of FCS, which is not acceptable.

rneatherway · 2015-11-27T17:51:45Z

Removes the project cracking from FCS.dll completely. Moves it into a new dll/exe pair that live in a new NuGet package. This breaks the FCS API, but a clean break.

rneatherway · 2015-11-27T17:53:25Z

This is largely working, but somehow I've run into the same problem with the type provider tests as before! Very frustrating! Last time it seemed to be fixed by https://github.com/rneatherway/FSharp.Compiler.Service/commit/9c577b2aa979c1ea3d96a4caddeedb12b1ff3889, but I'm not sure what is going on now.

7sharp9 · 2015-11-27T19:16:00Z

src/fsharp/FSharp.Compiler.Service.ProjectCracker/Program.fs

+        let opts = fmt.Deserialize(p.StandardOutput.BaseStream) :?> FSharp.Compiler.Service.ProjectCracker.Exe.ProjectOptions
+        p.WaitForExit()
+
+        convert opts, opts.LogOutput


Is the log output collected for each project cracked? Looks like this just collects the first one, not any sub projects log files.

You're right, it does. What would be better? Perhaps a Map from project filenames to log output?

Yeah that sounds fine, I can envisage problems where a referenced project
was causing an error which was masked due to only the parent log being
returned.

On 30 November 2015 at 14:29, Robin Neatherway notifications@github.com
wrote:

In src/fsharp/FSharp.Compiler.Service.ProjectCracker/Program.fs
#470 (comment)
:

arguments.Append(' ').Append(k).Append(' ').Append(v) |> ignore

let p = new System.Diagnostics.Process()

p.StartInfo.FileName <- Path.Combine(Path.GetDirectoryName(Reflection.Assembly.GetExecutingAssembly().Location),

"FSharp.Compiler.Service.ProjectCracker.Exe.exe")

p.StartInfo.Arguments <- arguments.ToString()

p.StartInfo.UseShellExecute <- false

p.StartInfo.CreateNoWindow <- true

p.StartInfo.RedirectStandardOutput <- true

ignore <| p.Start()

let fmt = new Serialization.Formatters.Binary.BinaryFormatter()

let opts = fmt.Deserialize(p.StandardOutput.BaseStream) :?> FSharp.Compiler.Service.ProjectCracker.Exe.ProjectOptions

p.WaitForExit()

convert opts, opts.LogOutput

You're right, it does. What would be better? Perhaps a Map from project
filenames to log output?

—
Reply to this email directly or view it on GitHub
https://github.com/fsharp/FSharp.Compiler.Service/pull/470/files#r46148480
.

dsyme · 2015-11-30T14:47:32Z

@rneatherway The errors are also strangely different in AppVeyor and Travis

dsyme · 2015-11-30T14:49:08Z

...FSharp.Compiler.Service.ProjectCracker.Exe/FSharp.Compiler.Service.ProjectCracker.Exe.fsproj

+    <Reference Include="Microsoft.Build.Utilities.v4.0" />
+    <Reference Include="mscorlib" />
+    <Reference Include="FSharp.Core, Version=$(TargetFSharpCoreVersion), Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a">
+      <Private>True</Private>


You have two FSHarp.Core references, one with Private true and one with Private false. I assume that is problematic?

AHA! Thanks so much for looking, that at least fixed the tests locally on Linux. I was scouring the project files looking for things like last time, but missed this somehow. Removing the Private true version was needed.

Previously only the top project was available. Also fix NuGet package

rneatherway · 2015-12-01T17:31:20Z

Still to do:

Update docs that mention checker.GetProjectOptionsFromProjectFile
Check that no other parts of the code use checker.GetProjectOptionsFromProjectFile
Be clear that new API is acceptable
Be clear that new NuGet package should or should not contain a v4.0 version, with possibly-degraded capabilities.

dsyme · 2015-12-01T18:01:59Z

No other parts use it. The API looks acceptable to me - any remaining concerns there?

dsyme · 2015-12-01T18:02:42Z

Re 4.0 - I think we should just remove it and see who complains. Whoever requires it can probably sit on the older nuget packages for a long time.

rneatherway · 2015-12-01T18:17:48Z

@7sharp9: This also fixes #455. I've had to put an ugly workaround for the Mono 4.2 xbuild bug. @DavidKarlas is that going to be backed out?

dsyme · 2015-12-01T19:11:15Z

@rneatherway Note #462 (comment) - Moving to Mono 4.2.1 has injected some failures into our tests. Is this what you're working around? thanks

DavidKarlas · 2015-12-01T20:04:20Z

@rneatherway Change that caused problems for F# in 4.2 was reverted for 4.2, before final 4.2 was released, so there should be no problems with 4.2...

rneatherway · 2015-12-01T20:46:20Z

@dsyme @DavidKarlas Mono 4.2 has changed the behaviour so that the ChildProjectReferences are no longer populated with the output file of the referenced projects (see this line: https://github.com/rneatherway/FSharp.Compiler.Service/commit/e06abd0db4d470b677417ab32a54286f637c11c1#diff-e9d0e05558936d4d3913fae7386cd6b6R102). Previously if Project2.fsproj referenced Project1.fsproj, then this is how we would obtain the reference to Project1.dll. Now we have to parse the subprojects and find the output file from them, which is why I added parsing of csproj as well (and should add vbproj too).

Should I be doing this in a different way for the new version of Mono?

dsyme · 2015-12-01T21:43:54Z

@rneatherway Any idea what's causing the AppVeyor errors? Looks like path normalization problems? I'd love to get this merged and I think we're quite close?

rneatherway · 2015-12-01T22:10:21Z

@dsyme I actually think it's a project configuration issue. When I compile the ProjectCrackerExe project on Windows with either MSbuild 12 or 14 I get:

"D:\dev\FSharp.Compiler.Service\src\fsharp\FSharp.Compiler.Service.ProjectCracker.Exe\FSharp.C
ompiler.Service.ProjectCracker.Exe.fsproj" (default target) (1) ->
(ResolveAssemblyReferences target) ->
  C:\Program Files (x86)\MSBuild\12.0\bin\Microsoft.Common.CurrentVersion.targets(1696,5): war
ning MSB3243: No way to resolve conflict between "Microsoft.Build, Version=12.0.0.0, Culture=n
eutral, PublicKeyToken=b03f5f7f11d50a3a" and "Microsoft.Build, Version=4.0.0.0, Culture=neutra
l, PublicKeyToken=b03f5f7f11d50a3a". Choosing "Microsoft.Build, Version=12.0.0.0, Culture=neut
ral, PublicKeyToken=b03f5f7f11d50a3a" arbitrarily. [D:\dev\FSharp.Compiler.Service\src\fsharp\
FSharp.Compiler.Service.ProjectCracker.Exe\FSharp.Compiler.Service.ProjectCracker.Exe.fsproj]
  C:\Program Files (x86)\MSBuild\12.0\bin\Microsoft.Common.CurrentVersion.targets(1696,5): war
ning MSB3243: No way to resolve conflict between "Microsoft.Build.Engine, Version=12.0.0.0, Cu
lture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "Microsoft.Build.Engine, Version=4.0.0.0,
Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". Choosing "Microsoft.Build.Engine, Version=1
2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" arbitrarily. [D:\dev\FSharp.Compile
r.Service\src\fsharp\FSharp.Compiler.Service.ProjectCracker.Exe\FSharp.Compiler.Service.Projec
tCracker.Exe.fsproj]
  C:\Program Files (x86)\MSBuild\12.0\bin\Microsoft.Common.CurrentVersion.targets(1696,5): war
ning MSB3243: No way to resolve conflict between "Microsoft.Build.Framework, Version=12.0.0.0,
 Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "Microsoft.Build.Framework, Version=4.0
.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". Choosing "Microsoft.Build.Framework,
Version=12.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" arbitrarily. [D:\dev\FShar
p.Compiler.Service\src\fsharp\FSharp.Compiler.Service.ProjectCracker.Exe\FSharp.Compiler.Servi
ce.ProjectCracker.Exe.fsproj]

rneatherway · 2015-12-01T22:27:13Z

Hmm, looks like there are duplicate entries further down. Must have been a dodgy merge at some point.

dsyme · 2015-12-02T00:23:34Z

Fantastic. Let's merge this?

dsyme · 2015-12-02T00:35:55Z

I'll merge this, let's clean up any further problems as we go.

Projects out of process (new api)

rneatherway added 7 commits November 22, 2015 17:35

wip

28455cd

wip

1fb9eae

further fixes

c9e1a83

Adjust all paths for new project names

0449432

Fix small mistakes in project files

adca6fc

FSharp.Core should be CopyLocal false

e7036e1

7sharp9 reviewed Nov 27, 2015
View reviewed changes

dsyme reviewed Nov 30, 2015
View reviewed changes

rneatherway added 4 commits November 30, 2015 16:13

Remove duplicate FSharp.Core

d54f3fa

Workaround for Mono 4.2 changes to xbuild

e06abd0

Add --text option to the exe for debugging

94e5425

Provide log output for all projects parsed

244d684

Previously only the top project was available. Also fix NuGet package

Minor file structure rationalisation

cc378fb

dsyme mentioned this pull request Dec 1, 2015

GetProjectOptionsFromProjectFile needs to run on reactor thread #335

Closed

Also parse referenced VB projects

0d75c08

Remove duplicate references from ProjectCrackerExe

1b17625

dsyme added a commit that referenced this pull request Dec 2, 2015

Merge pull request #470 from rneatherway/projects-out-of-process-new-api

2fd9b3d

Projects out of process (new api)

dsyme merged commit 2fd9b3d into fsharp:master Dec 2, 2015

dsyme mentioned this pull request Dec 2, 2015

Embed MSBUILD dependencies in the package #338

Closed

rneatherway deleted the projects-out-of-process-new-api branch December 2, 2015 09:31

This was referenced Dec 2, 2015

Discussion for implementing checker.GetProjectOptionsFromProjectFile in a separate process #371

Closed

How to include MSBUILD dependencies? #337

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Projects out of process (new api) #470

Projects out of process (new api) #470

rneatherway commented Nov 27, 2015

rneatherway commented Nov 27, 2015

rneatherway commented Nov 27, 2015

7sharp9 Nov 27, 2015

rneatherway Nov 30, 2015

7sharp9 Nov 30, 2015

dsyme commented Nov 30, 2015

dsyme Nov 30, 2015

rneatherway Nov 30, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

dsyme commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

DavidKarlas commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

rneatherway commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 2, 2015

dsyme commented Dec 2, 2015

Projects out of process (new api) #470

Projects out of process (new api) #470

Conversation

rneatherway commented Nov 27, 2015

rneatherway commented Nov 27, 2015

rneatherway commented Nov 27, 2015

7sharp9 Nov 27, 2015

Choose a reason for hiding this comment

rneatherway Nov 30, 2015

Choose a reason for hiding this comment

7sharp9 Nov 30, 2015

Choose a reason for hiding this comment

dsyme commented Nov 30, 2015

dsyme Nov 30, 2015

Choose a reason for hiding this comment

rneatherway Nov 30, 2015

Choose a reason for hiding this comment

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

dsyme commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

DavidKarlas commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 1, 2015

rneatherway commented Dec 1, 2015

rneatherway commented Dec 1, 2015

dsyme commented Dec 2, 2015

dsyme commented Dec 2, 2015